Stop NavBarAdapter's own selections from re-setting properties

Src/XCore/FlexUIAdapter/NavBarAdapter.cs

@@ -26,6 +26,7 @@ public class SidebarAdapter : AdapterBase, IxCoreColleague, IDisposable
 		private ChoiceGroupCollection m_choiceGroupCollectionCache; // cache of what was received by CreateUIForChoiceGroupCollection()
 		private List<ChoiceGroup> m_choiceGroupCache = new List<ChoiceGroup>(); // cache of what was received by CreateUIForChoiceGroup()
 		private string areasLabel; // label of the special case ChoiceGroup for Areas
+		private bool m_selectingInSidePane; // true while the adapter's own SelectTab/SelectItem runs
 
 		#region Disposable stuff
 		#if DEBUG
@@ -141,27 +142,50 @@ public void Init(Mediator mediator, PropertyTable propertyTable, XmlNode configu
 		}
 
 		/// <summary>
-		/// Handle when an item on the sidepane is clicked.
+		/// Handle when an item on the sidepane is clicked. A selection the adapter itself makes
+		/// while reflecting a property change raises the same event and is ignored here.
 		/// </summary>
 		private void SidePaneItemClickedHandler(Item itemClicked)
 		{
+			if (m_selectingInSidePane)
+				return;
 			var choice = itemClicked.Tag as ChoiceBase;
 			Debug.Assert(choice != null, "SidePaneItemClickedHandler received an Item with an invalid Tag.");
 			if (choice != null)
 				choice.OnClick(this, null);
 		}
 
 		/// <summary>
-		/// Handle when a tab on the sidepane is clicked.
+		/// Handle when a tab on the sidepane is clicked. A selection the adapter itself makes
+		/// while reflecting a property change raises the same event and is ignored here.
 		/// </summary>
 		private void SidePaneTabClickedHandler(Tab tabClicked)
 		{
+			if (m_selectingInSidePane)
+				return;
 			var choice = tabClicked.Tag as ChoiceBase;
 			Debug.Assert(choice != null, "SidePaneTabClickedHandler received a Tab with an invalid Tag.");
 			if (choice != null)
 				choice.OnClick(this, null);
 		}
 
+		/// <summary>
+		/// Runs one of the side pane's selection calls with the click handlers silenced, so
+		/// that showing a property's value does not set that property again.
+		/// </summary>
+		private bool SelectInSidePane(Func<bool> select)
+		{
+			m_selectingInSidePane = true;
+			try
+			{
+				return select();
+			}
+			finally
+			{
+				m_selectingInSidePane = false;
+			}
+		}
+
 		public override void PersistLayout()
 		{
 			// use MyPanel
@@ -555,12 +579,12 @@ public void OnPropertyChanged(string propertyName)
 									// overwriting the property table value.  See FWR-2004.
 									string propToolForArea = "ToolForAreaNamed_" + areaName;
 									string toolForArea = m_propertyTable.GetStringProperty(propToolForArea, null);
-									m_sidepane.SelectTab(tab);
+									SelectInSidePane(() => m_sidepane.SelectTab(tab));
 									// FWR-2895 Deleting a Custom list could result in needing to
 									// update the PropertyTable.
 									if (!String.IsNullOrEmpty(toolForArea) && !toolForArea.StartsWith("?"))
 									{
-										var fsuccess = m_sidepane.SelectItem(tab, toolForArea);
+										var fsuccess = SelectInSidePane(() => m_sidepane.SelectItem(tab, toolForArea));
 										if (!fsuccess)
 										{
 											m_propertyTable.SetProperty(propToolForArea, null, true);
@@ -605,7 +629,7 @@ private void SetToolForCurrentArea(string tool)
 						// calling the SelectItem if it is already the CurrentItem.
 						if(m_sidepane.CurrentItem != item)
 						{
-							m_sidepane.SelectItem(tab, item.Name);
+							SelectInSidePane(() => m_sidepane.SelectItem(tab, item.Name));
 						}
 						break;
 					}

Src/XCore/xCoreTests/SidebarAdapterSelectionTests.cs

@@ -0,0 +1,189 @@
+// Copyright (c) 2026 SIL International
+// This software is licensed under the LGPL, version 2.1 or later
+// (http://www.gnu.org/licenses/lgpl-2.1.html)
+
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Windows.Forms;
+using System.Xml;
+using NUnit.Framework;
+using SIL.SilSidePane;
+
+namespace XCore
+{
+	/// <summary>
+	/// How <see cref="SidebarAdapter"/> reflects areaChoice and ToolForAreaNamed_ changes in
+	/// the side pane without setting those properties again.
+	/// </summary>
+	[TestFixture]
+	[Apartment(ApartmentState.STA)]
+	public class SidebarAdapterSelectionTests
+	{
+		/// <summary>
+		/// Two areas with two tools each, in the shape of the sidebar section of Main.xml.
+		/// </summary>
+		private const string SidebarXml =
+			"<root>"
+			+ "<lists>"
+			+ "<list id=\"AreasList\">"
+			+ "<item label=\"Lexicon\" value=\"lexicon\"><parameters><panels>"
+			+ "<listPanel label=\"Lexical Tools\" listId=\"LexicalToolsList\"/></panels></parameters></item>"
+			+ "<item label=\"Grammar\" value=\"grammar\"><parameters><panels>"
+			+ "<listPanel label=\"Grammar Tools\" listId=\"GrammarToolsList\"/></panels></parameters></item>"
+			+ "</list>"
+			+ "<list id=\"LexicalToolsList\">"
+			+ "<item label=\"Lexicon Edit\" value=\"lexiconEdit\"><control/></item>"
+			+ "<item label=\"Browse\" value=\"lexiconBrowse\"><control/></item>"
+			+ "</list>"
+			+ "<list id=\"GrammarToolsList\">"
+			+ "<item label=\"Category Edit\" value=\"posEdit\"><control/></item>"
+			+ "<item label=\"Category Browse\" value=\"posBrowse\"><control/></item>"
+			+ "</list>"
+			+ "</lists>"
+			+ "<sidebar>"
+			+ "<tab label=\"Areas\" list=\"AreasList\" behavior=\"singlePropertyAtomicValue\" property=\"areaChoice\"/>"
+			+ "<tab label=\"Lexical Tools\" list=\"LexicalToolsList\" behavior=\"singlePropertyAtomicValue\""
+			+ " property=\"currentContentControl\"/>"
+			+ "<tab label=\"Grammar Tools\" list=\"GrammarToolsList\" behavior=\"singlePropertyAtomicValue\""
+			+ " property=\"currentContentControl\"/>"
+			+ "</sidebar>"
+			+ "</root>";
+
+		private Form m_window;
+		private Mediator m_mediator;
+		private PropertyTable m_propertyTable;
+		private ImageCollection m_smallImages;
+		private ImageCollection m_largeImages;
+		private SidebarAdapter m_adapter;
+		private Control m_panel;
+		private ChoiceGroupCollection m_groups;
+
+		[SetUp]
+		public void SetUp()
+		{
+			m_window = new Form();
+			m_mediator = new Mediator();
+			m_propertyTable = new PropertyTable(m_mediator);
+			m_smallImages = new ImageCollection(false);
+			m_largeImages = new ImageCollection(true);
+			m_adapter = new SidebarAdapter();
+			m_panel = m_adapter.Init(m_window, m_smallImages, m_largeImages, m_mediator, m_propertyTable);
+			m_window.Controls.Add(m_panel);
+
+			var doc = new XmlDocument();
+			doc.LoadXml(SidebarXml);
+			m_groups = new ChoiceGroupCollection(m_mediator, m_propertyTable, m_adapter,
+				doc.DocumentElement.SelectSingleNode("sidebar"));
+			m_groups.Init();
+			m_adapter.FinishInit();
+			CreateHandles(m_window);
+		}
+
+		[TearDown]
+		public void TearDown()
+		{
+			m_adapter.Dispose();
+			m_window.Dispose();
+			m_smallImages.Dispose();
+			m_largeImages.Dispose();
+			m_propertyTable.Dispose();
+			m_mediator.Dispose();
+		}
+
+		/// <summary>
+		/// The side pane's list views raise their selection events only once they have a
+		/// window handle, which a hidden child of an unshown form never gets on its own.
+		/// </summary>
+		private static void CreateHandles(Control control)
+		{
+			var unused = control.Handle;
+			foreach (Control child in control.Controls)
+				CreateHandles(child);
+		}
+
+		private static IEnumerable<Control> AllControls(Control root)
+		{
+			yield return root;
+			foreach (Control child in root.Controls)
+				foreach (var descendant in AllControls(child))
+					yield return descendant;
+		}
+
+		/// <summary>
+		/// Every side-pane item widget, across all the tabs' item lists. The form is never
+		/// shown, so a list's Visible flag cannot tell which tab is current.
+		/// </summary>
+		private IEnumerable<ListViewItem> AllItemWidgets()
+		{
+			return AllControls(m_panel).OfType<ListView>().SelectMany(list => list.Items.Cast<ListViewItem>());
+		}
+
+		/// <summary>
+		/// The tool selected in the item list that holds the named tool, or null when none is.
+		/// Each tab's list keeps its own selection, so the list is named by one of its tools.
+		/// </summary>
+		private string SelectedToolInTheListHolding(string tool)
+		{
+			var list = AllItemWidgets().First(widget => ((Item)widget.Tag).Name == tool).ListView;
+			if (list.SelectedItems.Count == 0)
+				return null;
+			return ((ListPropertyChoice)((Item)list.SelectedItems[0].Tag).Tag).Value;
+		}
+
+		private void ShowArea(string area, string rememberedTool)
+		{
+			m_propertyTable.SetProperty("areaChoice", area, false);
+			m_propertyTable.SetProperty("ToolForAreaNamed_" + area, rememberedTool, false);
+			m_adapter.OnPropertyChanged("areaChoice");
+		}
+
+		[Test]
+		public void AreaChoiceChange_SelectsTheRememberedToolWithoutSettingAnyProperty()
+		{
+			ShowArea("grammar", "posBrowse");
+
+			Assert.That(SelectedToolInTheListHolding("posBrowse"), Is.EqualTo("posBrowse"));
+			Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property");
+			Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False);
+			Assert.That(m_propertyTable.PropertyExists("areaChoiceParameters"), Is.False);
+		}
+
+		[Test]
+		public void ReturnToAnArea_SelectsTheToolRememberedSinceTheLastVisit()
+		{
+			ShowArea("lexicon", "lexiconEdit");
+			ShowArea("grammar", "posEdit");
+
+			ShowArea("lexicon", "lexiconBrowse");
+
+			Assert.That(SelectedToolInTheListHolding("lexiconBrowse"), Is.EqualTo("lexiconBrowse"));
+			Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property");
+			Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False);
+		}
+
+		[Test]
+		public void ToolForAreaChange_SelectsThatToolWithoutSettingAnyProperty()
+		{
+			ShowArea("lexicon", "lexiconEdit");
+			m_propertyTable.SetProperty("ToolForAreaNamed_lexicon", "lexiconBrowse", false);
+
+			m_adapter.OnPropertyChanged("ToolForAreaNamed_lexicon");
+
+			Assert.That(SelectedToolInTheListHolding("lexiconBrowse"), Is.EqualTo("lexiconBrowse"));
+			Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property");
+			Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False);
+		}
+
+		[Test]
+		public void ItemSelectedOutsideTheAdapter_StillSetsTheProperty()
+		{
+			ShowArea("lexicon", "lexiconEdit");
+
+			AllItemWidgets().First(widget => ((Item)widget.Tag).Name == "lexiconBrowse").Selected = true;
+
+			Assert.That(m_propertyTable.GetStringProperty("currentContentControl", null), Is.EqualTo("lexiconBrowse"));
+			Assert.That(m_mediator.JobItems, Is.GreaterThan(0), "a selection the adapter did not make is a click");
+		}
+	}
+}