Fix LT-22316 regression: persist hidden column labels

Src/Common/Controls/XMLViews/BrowseViewer.cs

@@ -23,6 +23,7 @@
 using System.ComponentModel;
 using System.Diagnostics;
 using System.Drawing;
+using System.Security;
 using System.Text;
 using System.Windows.Forms;
 using System.Xml;
@@ -2968,7 +2969,7 @@ protected void InsertColumn(XmlNode colSpec, int position)
 
 		// Note: often we also want to update LayoutCache.LayoutVersionNumber.
 		// (last updated by Ariel Rorabaugh, Oct 28, 2025, for adding PictureLicense column to Browse view)
-		internal const int kBrowseViewVersion = 19;
+		internal const int kBrowseViewVersion = 20;
 
 		/// <summary>
 		/// Column has been added or removed, update all child windows.
@@ -3042,6 +3043,33 @@ protected void UpdateColumnList()
 					colList.Append(node.OuterXml);
 				}
 			}
+			// Save labels of hidden columns so we can distinguish "removed by user" from "new column" on load.
+			// Use SortedSet with Ordinal comparison for deterministic output (stable for Send/Receive).
+			var activeLabels = new HashSet<string>();
+			foreach (XmlNode node in ColumnSpecs)
+			{
+				var label = XmlUtils.GetOptionalAttributeValue(node, "label", "");
+				if (!string.IsNullOrEmpty(label))
+					activeLabels.Add(label);
+			}
+			var hiddenLabels = new SortedSet<string>(StringComparer.Ordinal);
+			foreach (XmlNode node in m_xbv.Vc.PossibleColumnSpecs)
+			{
+				var label = XmlUtils.GetOptionalAttributeValue(node, "label", "");
+				if (!string.IsNullOrEmpty(label) && !activeLabels.Contains(label))
+					hiddenLabels.Add(label);
+			}
+			if (hiddenLabels.Count == 0)
+			{
+				// Write a sentinel so the load side knows hidden tracking is active
+				// (distinguishes "all columns visible" from "pre-upgrade save with no tracking").
+				colList.Append("<hidden/>");
+			}
+			else
+			{
+				foreach (var label in hiddenLabels)
+					colList.Append("<hidden label=\"" + SecurityElement.Escape(label) + "\"/>");
+			}
 			colList.Append("</root>");
 			m_propertyTable.SetProperty(m_xbv.Vc.ColListId, colList.ToString(), PropertyTable.SettingsGroup.LocalSettings, true);
 		}

Src/Common/Controls/XMLViews/XMLViewsTests/TestXmlBrowseView.cs

@@ -3,6 +3,7 @@
 // (http://www.gnu.org/licenses/lgpl-2.1.html)
 
 using System;
+using System.Collections.Generic;
 using System.Xml;
 using NUnit.Framework;
 using SIL.FieldWorks.Common.Controls;
@@ -133,6 +134,84 @@ public void MigrateBrowseColumns()
 			}
 		}
 
+		[Test]
+		public void MigrateBrowseColumns_Version19ToVersion20()
+		{
+			var input =
+				"<root version=\"19\">" +
+				"<column layout=\"EntryHeadwordForFindEntry\" label=\"Headword\" sortmethod=\"FullSortKey\" ws=\"$ws=vernacular\" editable=\"false\" width=\"96000\"/>" +
+				"<column layout=\"Unknown Test\"/>" +
+				"</root>";
+			using (var mediator = new Mediator())
+			using (var propertyTable = new PropertyTable(mediator))
+			{
+				var output = XmlBrowseViewBaseVc.GetSavedColumns(input, mediator, propertyTable, "myKey");
+				Assert.That(XmlUtils.GetOptionalAttributeValue(output.DocumentElement, "version"), Is.EqualTo(BrowseViewer.kBrowseViewVersion.ToString()));
+				Assert.That(XmlUtils.GetOptionalAttributeValue(output.DocumentElement, "version"), Is.EqualTo("20"));
+				// Columns should be preserved as-is
+				var headwordNode = output.SelectSingleNode("//column[@label='Headword']");
+				Assert.That(headwordNode, Is.Not.Null);
+				Assert.That(XmlUtils.GetOptionalAttributeValue(headwordNode, "layout"), Is.EqualTo("EntryHeadwordForFindEntry"));
+			}
+		}
+
+		[Test]
+		public void MigrateBrowseColumns_HiddenElementsPreservedThroughMigration()
+		{
+			// Version 19 with hidden elements (simulating a future scenario where hidden elements exist)
+			var input =
+				"<root version=\"19\">" +
+				"<column layout=\"EntryHeadwordForFindEntry\" label=\"Headword\"/>" +
+				"<hidden label=\"Lexeme Form\"/>" +
+				"</root>";
+			using (var mediator = new Mediator())
+			using (var propertyTable = new PropertyTable(mediator))
+			{
+				var output = XmlBrowseViewBaseVc.GetSavedColumns(input, mediator, propertyTable, "myKey");
+				Assert.That(XmlUtils.GetOptionalAttributeValue(output.DocumentElement, "version"), Is.EqualTo("20"));
+				// Hidden elements should be preserved
+				var hiddenNode = output.SelectSingleNode("//hidden[@label='Lexeme Form']");
+				Assert.That(hiddenNode, Is.Not.Null, "Hidden column labels should be preserved through migration");
+			}
+		}
+
+		[Test]
+		public void HiddenSentinel_AllColumnsVisible_AllowsAutoAddOfNewCommonColumns()
+		{
+			// Simulate a version-20 save where all columns are visible: the sentinel <hidden/>
+			// element (no label) signals that hidden tracking is active, so new common columns
+			// should be auto-added rather than skipped by the bootstrap path.
+			var savedXml =
+				"<root version=\"20\">" +
+				"<column layout=\"Name\" label=\"Name\" ws=\"$ws=best analysis\" field=\"Name\"/>" +
+				"<column layout=\"Abbreviation\" label=\"Abbreviation\" ws=\"$ws=best analysis\" field=\"Abbreviation\"/>" +
+				"<hidden/>" +  // sentinel: hidden tracking active, but nothing hidden
+				"</root>";
+			var doc = new XmlDocument();
+			doc.LoadXml(savedXml);
+
+			var hiddenNodes = doc.DocumentElement.SelectNodes("//hidden");
+			bool hasHiddenTracking = hiddenNodes.Count > 0;
+			Assert.That(hasHiddenTracking, Is.True, "Sentinel <hidden/> should enable hidden tracking");
+
+			// Collect hidden labels — sentinel has no label, so hiddenLabels should be empty
+			var hiddenLabels = new HashSet<string>();
+			foreach (XmlNode hidden in hiddenNodes)
+			{
+				var label = XmlUtils.GetOptionalAttributeValue(hidden, "label", "");
+				if (!string.IsNullOrEmpty(label))
+					hiddenLabels.Add(label);
+			}
+			Assert.That(hiddenLabels, Is.Empty, "Sentinel <hidden/> should not contribute a label");
+
+			// A new common column that is not in the saved list and not hidden should be auto-added
+			// (i.e. hasHiddenTracking is true AND label is not in hiddenLabels)
+			var newColumnLabel = "NewCommonColumn";
+			Assert.That(hasHiddenTracking, Is.True);
+			Assert.That(hiddenLabels.Contains(newColumnLabel), Is.False,
+				"New column should not be blocked when hidden tracking is active with no hidden labels");
+		}
+
 		void VerifyColumn(XmlNode output, string layoutName, string attrName, string attrVal)
 		{
 			var node = output.SelectSingleNode("//column[@layout='" + layoutName + "']");

Src/Common/Controls/XMLViews/XmlBrowseViewBaseVc.cs

@@ -229,10 +229,34 @@ public XmlBrowseViewBaseVc(XmlNode xnSpec, int fakeFlid, XmlBrowseViewBase xbv)
 						ColumnSpecs.Add(node);
 				}
 
+				// Collect labels of columns the user deliberately removed (hidden).
+				var hiddenNodes = doc.DocumentElement.SelectNodes("//hidden");
+				var hiddenLabels = new HashSet<string>();
+				bool hasHiddenTracking = hiddenNodes.Count > 0;
+				if (hasHiddenTracking)
+				{
+					foreach (XmlNode hidden in hiddenNodes)
+					{
+						var label = XmlUtils.GetOptionalAttributeValue(hidden, "label", "");
+						if (!string.IsNullOrEmpty(label))
+							hiddenLabels.Add(label);
+					}
+				}
+
 				foreach (var node in newPossibleColumns)
 				{
-					// add any possible columns that were not in the saved list and are common (and not custom)
-					if (!IsCustomField(node, out _) && XmlUtils.GetOptionalAttributeValue(node, "common", "false") == "true")
+					if (!hasHiddenTracking)
+					{
+						// Bootstrap: no hidden tracking yet (pre-upgrade save).
+						// Don't auto-add anything — all missing columns are presumed
+						// deliberately removed. They'll be properly tracked on next save.
+						continue;
+					}
+					// Add common non-custom columns that were not in the saved list and not deliberately hidden.
+					var label = XmlUtils.GetOptionalAttributeValue(node, "label", "");
+					if (!hiddenLabels.Contains(label)
+						&& !IsCustomField(node, out _)
+						&& XmlUtils.GetOptionalAttributeValue(node, "common", "false") == "true")
 					{
 						ColumnSpecs.Add(node);
 					}
@@ -299,6 +323,9 @@ internal static XmlDocument GetSavedColumns(string savedCols, Mediator mediator,
 						case 18:
 							savedCols = FixVersion19Columns(savedCols);
 							savedCols = savedCols.Replace("root version=\"18\"", "root version=\"19\"");
+							goto case 19;
+						case 19:
+							savedCols = savedCols.Replace("root version=\"19\"", "root version=\"20\"");
 							propertyTable.SetProperty(colListId, savedCols, true);
 							doc.LoadXml(savedCols);
 							break;
@@ -613,23 +640,31 @@ internal int ListItemsClass
 		/// </summary>
 		internal bool IsValidColumnSpec(XmlNode colSpec)
 		{
-			if (GetPartFromParentNode(colSpec, ListItemsClass) == null)
-			{
-				return false;
-			}
 			// If it is a Custom Field, check that it is valid and has the correct label.
 			if (IsCustomField(colSpec, out var isValid))
 			{
 				return isValid;
 			}
-			// In the simple case, `node`s label should match a label in PossibleColumnSpecs. There may be more complicated cases.
-			// ENHANCE (Hasso) 2025.11: 'layout' (mandatory?) and 'field' (optional) would be better attributes to match, but that would require more test setup.
+			// If the column matches a known PossibleColumnSpec by label, it is valid.
+			// Check this before GetPartFromParentNode because some views (e.g. Phonological Features
+			// with FsFeatDefn) have valid columns whose class hierarchy has no matching parts in
+			// the part inventory; GetPartFromParentNode would incorrectly reject those columns.
 			var label = XmlUtils.GetLocalizedAttributeValue(colSpec, "label", null) ??
 						XmlUtils.GetMandatoryAttributeValue(colSpec, "label");
 			var originalLabel = XmlUtils.GetLocalizedAttributeValue(colSpec, "originalLabel", null) ??
 								XmlUtils.GetAttributeValue(colSpec, "originalLabel");
-			return XmlViewsUtils.FindNodeWithAttrVal(PossibleColumnSpecs, "label", label) != null ||
-					XmlViewsUtils.FindNodeWithAttrVal(PossibleColumnSpecs, "label", originalLabel) != null;
+			if (XmlViewsUtils.FindNodeWithAttrVal(PossibleColumnSpecs, "label", label) != null ||
+				XmlViewsUtils.FindNodeWithAttrVal(PossibleColumnSpecs, "label", originalLabel) != null)
+			{
+				return true;
+			}
+			// For columns not in PossibleColumnSpecs (e.g. generated columns with a layout
+			// attribute), fall back to checking whether the part/layout inventory can resolve
+			// the column. Columns without a layout attribute that aren't in PossibleColumnSpecs
+			// are invalid (GetPartFromParentNode would trivially return the node itself).
+			if (XmlUtils.GetOptionalAttributeValue(colSpec, "layout") == null)
+				return false;
+			return GetPartFromParentNode(colSpec, ListItemsClass) != null;
 		}
 
 		/// <summary>