LT-22691: Let a menu authority answer a whole menu id, submenus included

Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs

@@ -310,7 +310,7 @@ private static Func<string, bool> GhostSuccessorMatcher(string ghostAutomationId
 			if (string.IsNullOrEmpty(ghostAutomationId))
 				return null;
 
-			const string marker = "/ghost";
+			const string marker = DetailField.GhostStableIdSuffix;
 			var markerIndex = ghostAutomationId.IndexOf(marker, StringComparison.Ordinal);
 			if (markerIndex < 0)
 				return null;

Src/Common/FwAvalonia/Detail/DetailModel.cs

@@ -1511,6 +1511,18 @@ public DetailLinkRequest(DetailField field, DetailChooserLink link)
 	/// </summary>
 	public sealed class DetailField
 	{
+		/// <summary>
+		/// The marker the composer appends to the stable id of a ghost row, the prompt that
+		/// stands in for an empty sequence. The row's node is the id without it.
+		/// </summary>
+		public const string GhostStableIdSuffix = "/ghost";
+
+		/// <summary>
+		/// The marker the composer puts in the stable id of a custom-field row, which it
+		/// builds while walking an object rather than from the compiled model.
+		/// </summary>
+		public const string CustomFieldStableIdMarker = "/custom:";
+
 		public DetailField(
 			string stableId,
 			string label,

Src/XCore/xCoreInterfaces/ChoiceGroup.cs

@@ -427,6 +427,11 @@ public string ListId
 			}
 		}
 		protected override void Populate()
+		{
+			Populate(querySubmenuVisibility: true);
+		}
+
+		private void Populate(bool querySubmenuVisibility)
 		{
 			Clear();
 			if (IsAListGroup)
@@ -437,12 +442,12 @@ protected override void Populate()
 			{
 				foreach (XmlNode n in m_configurationNodes)
 				{
-					Populate(n);
+					Populate(n, querySubmenuVisibility);
 				}
 			}
 			else
 			{
-				Populate(m_configurationNode);
+				Populate(m_configurationNode, querySubmenuVisibility);
 			}
 		}
 
@@ -455,6 +460,16 @@ public void PopulateNow()
 			Populate();
 		}
 
+		/// <summary>
+		/// Populates the group, keeping every nested submenu when
+		/// <paramref name="querySubmenuVisibility"/> is false instead of asking the colleagues
+		/// whether one of its items is visible; the caller then decides the submenu's fate.
+		/// </summary>
+		public void PopulateNow(bool querySubmenuVisibility)
+		{
+			Populate(querySubmenuVisibility);
+		}
+
 		protected void PopulateFromList()
 		{
 			/// Just before this group is displayed, allow the group's contents to be modified by colleagues
@@ -521,6 +536,11 @@ public bool HasSubGroups()
 		}
 
 		protected void Populate(XmlNode node)
+		{
+			Populate(node, querySubmenuVisibility: true);
+		}
+
+		private void Populate(XmlNode node, bool querySubmenuVisibility)
 		{
 			Debug.Assert( node != null);
 			XmlNodeList items = node.SelectNodes("item | menu | group");
@@ -534,11 +554,11 @@ protected void Populate(XmlNode node)
 						break;
 					case "menu":
 						ChoiceGroup group = new ChoiceGroup(m_mediator, m_propertyTable, m_adapter, childNode, this);
-						group.Populate(childNode);
+						group.Populate(childNode, querySubmenuVisibility);
 						//Only add the submenu if it contains a list of items what will be visible.
 						//We do not want an empty submenu  LT-8791.
 						string hasList = XmlUtils.GetAttributeValue(childNode, "list");
-						if (hasList != null || ASubmenuItemIsVisible(group))
+						if (hasList != null || !querySubmenuVisibility || ASubmenuItemIsVisible(group))
 							this.Add(group);
 						break;
 					case "group":	//for tree views in the sidebar

Src/xWorks/Avalonia/Composer/DetailComposer.cs

@@ -870,7 +870,7 @@ private ViewNode MakeCustomFieldNode(ViewNode placeholder, int flid)
 						break;
 				}
 
-				return new ViewNode($"{placeholder.StableId}/custom:{fieldName}", ViewNodeKind.Field,
+				return new ViewNode(placeholder.StableId + DetailField.CustomFieldStableIdMarker + fieldName, ViewNodeKind.Field,
 					_mdc.GetFieldLabel(flid), null, fieldName, rawEditor, EditorClassification.Known,
 					wsSpec, ViewVisibility.Always, ViewExpansion.NotApplicable, placeholder.Indented,
 					null, null, menuId: "mnuDataTree-Help");
@@ -3017,7 +3017,7 @@ private void AddGhostPrompt(ViewNode node, ICmObject obj, int depth)
 				var prompt = string.Format(
 					SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaStrings.GhostAddPromptFormat, label);
 
-				var stableId = $"{StableId(node, obj)}/ghost";
+				var stableId = StableId(node, obj) + DetailField.GhostStableIdSuffix;
 				var ghost = ResolveGhostCreation(node, obj);
 				AddField(new DetailField(stableId, label, node.Field,
 					node.WritingSystem, DetailFieldKind.Text, node.EditorClassification,

Src/xWorks/Avalonia/Hosting/IDetailMenuAuthority.cs

@@ -11,11 +11,16 @@ namespace SIL.FieldWorks.XWorks
 	/// The complete native answer for every leaf of the context-menu ids it owns: display
 	/// (visible, enabled, checked, label) and execution together, computed from the Avalonia
 	/// row alone. Nothing on the mediator, the hidden DataTree command adapter included, takes
-	/// part in an owned id.
+	/// part in an owned id. An owned submenu shows, with its configured label, whenever any of
+	/// its leaves does; no colleague can hide or relabel it.
 	/// </summary>
 	public interface IDetailMenuAuthority
 	{
-		/// <summary>Whether this authority answers every leaf under the given menu id.</summary>
+		/// <summary>
+		/// Whether this authority answers every leaf under the given menu id, submenus
+		/// included. Asked only for the ids a menu is built from; a nested menu is never
+		/// offered on its own.
+		/// </summary>
 		bool Owns(string menuId);
 
 		/// <summary>

Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs

@@ -624,12 +624,44 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField
 			// Show all right now never dispatches or persists: it only marks the row for the
 			// host's transient reveal.
 			registry.Add("CmdDataTree-WritingSystemMenu-ShowAllRightNow",
-				(c, d) => ShowAllWritingSystemsItem(d, field));
+				(c, d) => ShowAllWritingSystemsItem(LabelOf(d), field));
 
-			var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId);
-			// Locate the clicked node in the field's OWN compiled model (with any current override
-			// already applied), so visibility checkmarks and move enablement reflect the live state.
-			ViewNodeLocation location = null;
+			// Unknown/stale target: leave the field commands on mediator dispatch rather than
+			// guess.
+			if (!TryLocateOverrideTarget(field, out var templateId, out var location))
+				return;
+			registry.Add("CmdAlwaysVisible",
+				(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Always));
+			registry.Add("CmdIfData",
+				(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.IfData));
+			registry.Add("CmdNormallyHidden",
+				(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Never));
+			registry.Add("CmdDataTree-MoveFieldUp",
+				(c, d) => MoveItem(LabelOf(d), field, location, up: true));
+			registry.Add("CmdDataTree-MoveFieldDown",
+				(c, d) => MoveItem(LabelOf(d), field, location, up: false));
+		}
+
+		private static string LabelOf(UIItemDisplayProperties display)
+			=> XCoreMenuBridge.StripAccelerator(display.Text);
+
+		/// <summary>
+		/// Locates the row's node in its own compiled model, with the current override applied,
+		/// so visibility checkmarks and move enablement reflect the live state. False without a
+		/// log when the row can never be a target: no class, layout or override store, or a row
+		/// the composer synthesized with no node of its own. False with the reason logged when
+		/// the compile fails or the model has no node for the row's template id.
+		/// </summary>
+		internal bool TryLocateOverrideTarget(DetailField field, out string templateId,
+			out ViewNodeLocation location)
+		{
+			templateId = null;
+			location = null;
+			if (field == null || string.IsNullOrEmpty(field.ClassName) || string.IsNullOrEmpty(field.LayoutName)
+				|| ViewOverrideStore == null || !TryTemplateIdOf(field.StableId, out templateId))
+			{
+				return false;
+			}
 			try
 			{
 				if (Cache.ServiceLocator.ObjectRepository.TryGetObject(field.ObjectHvo, out var fieldObj))
@@ -642,45 +674,52 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField
 			}
 			catch (Exception e)
 			{
-				Logger.WriteError("Resolving the field's override target failed; this row's "
-					+ "menu-button commands fall back to ordinary command dispatch.", e);
-				return;
+				Logger.WriteError("Resolving the field's override target failed; its Field Visibility "
+					+ "and Move Field commands are not retargeted to the override layer.", e);
+				return false;
 			}
-
-			// Unknown/stale target: leave the field commands on the legacy path rather than
-			// guess.
-			if (location != null)
+			if (location == null)
 			{
-				registry.Add("CmdAlwaysVisible",
-					(c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.Always));
-				registry.Add("CmdIfData",
-					(c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.IfData));
-				registry.Add("CmdNormallyHidden",
-					(c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.Never));
-				registry.Add("CmdDataTree-MoveFieldUp",
-					(c, d) => MoveItem(d, field, location, up: true));
-				registry.Add("CmdDataTree-MoveFieldDown",
-					(c, d) => MoveItem(d, field, location, up: false));
+				Logger.WriteEvent(string.Format("Detail row '{0}' has no node in its compiled model; its "
+					+ "Field Visibility and Move Field commands are not retargeted to the override layer.",
+					templateId));
+				return false;
 			}
+			return true;
+		}
+
+		// The node a row's stable id names; false for a synthesized row with no node of its
+		// own (an item or relation path after the hvo, or a custom field). A ghost row stands
+		// in for its empty sequence node.
+		private static bool TryTemplateIdOf(string stableId, out string templateId)
+		{
+			templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(stableId);
+			if (templateId.Contains(DetailField.CustomFieldStableIdMarker))
+				return false;
+			var at = stableId.IndexOf('@');
+			var path = at < 0 ? -1 : stableId.IndexOf('/', at);
+			if (path < 0)
+				return true;
+			if (!stableId.Substring(path).StartsWith(DetailField.GhostStableIdSuffix, StringComparison.Ordinal))
+				return false;
+			templateId = stableId.Substring(0, at);
+			return true;
 		}
 
 		// A Field Visibility menu item: checked when it is the field's current visibility, executes the
 		// SetVisibility override mutation (idempotent -- re-choosing the current value is a
 		// harmless write).
-		private DetailMenuItem VisibilityItem(UIItemDisplayProperties display, DetailField field,
+		private DetailMenuItem VisibilityItem(string label, DetailField field,
 			string templateId, ViewNodeLocation location, ViewVisibility target)
 		{
-			var label = XCoreMenuBridge.StripAccelerator(display.Text);
 			var isChecked = location.Visibility == target;
 			return new DetailMenuItem(label, isEnabled: true, isChecked: isChecked, children: null,
 				execute: () => ApplyFieldVisibility(field, templateId, target));
 		}
 
 		// A Move Field item: disabled at the first sibling (up) / last sibling (down) / when alone.
-		private DetailMenuItem MoveItem(UIItemDisplayProperties display, DetailField field,
-			ViewNodeLocation location, bool up)
+		private DetailMenuItem MoveItem(string label, DetailField field, ViewNodeLocation location, bool up)
 		{
-			var label = XCoreMenuBridge.StripAccelerator(display.Text);
 			var canMove = up ? location.CanMoveUp : location.CanMoveDown;
 			return new DetailMenuItem(label, isEnabled: canMove, isChecked: false, children: null,
 				execute: canMove ? (Action)(() => ApplyMoveField(field, location, up)) : null);
@@ -692,9 +731,8 @@ private DetailMenuItem MoveItem(UIItemDisplayProperties display, DetailField fie
 		/// record) and recomposes. The reveal is view state, not a command, so the item
 		/// dispatches nothing and never writes the override.
 		/// </summary>
-		private DetailMenuItem ShowAllWritingSystemsItem(UIItemDisplayProperties display, DetailField field)
-			=> new DetailMenuItem(XCoreMenuBridge.StripAccelerator(display.Text), isEnabled: true,
-				isChecked: false, children: null, execute: () =>
+		private DetailMenuItem ShowAllWritingSystemsItem(string label, DetailField field)
+			=> new DetailMenuItem(label, isEnabled: true, isChecked: false, children: null, execute: () =>
 				{
 					m_showAllWsFields.Add(ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId));
 					RefreshAvaloniaDetail();