2040852 - macOS: Honor Gecko command-disabled state for standard Edit menu items via MacApplicationDelegate.validateUserInterfaceItem:

Status: NEW

(Core :: Widget: Cocoa, defect)

Description

[Stephen A Pohl [:spohl]]

Follow-up to bug 2040851.

Bug 2040851 will land the SetEnabled fix in widget/cocoa/nsMenuItemX.mm so the standard Edit menu items (cut:, copy:, paste:, undo:, redo:, delete:, selectAll:) are unconditionally enabled in AppKit's view -- the XUL <command> element's disabled attribute is no longer mirrored to the NSMenuItem for these items. AppKit's automatic validateUserInterfaceItem: walk on the responder chain then determines the actual enable/disable state at menu-rendering time:

• When a native sheet is up, the sheet's first-responder NSText answers YES/NO based on its selection / clipboard state. Correct.
• When focus is in Gecko, the chain walks past ChildView (no implementations) up to MacApplicationDelegate, whose forwarders implement the selectors. NSResponder's default validateUserInterfaceItem: returns YES whenever the responder responds to the selector -- so the menu items appear enabled regardless of whether Gecko has anything to cut / copy / paste / etc. Clicking just no-ops via menuItemHit: → cmd_X on the focused element that has nothing to act on.

Functionally fine (no broken shortcuts) and the current state is strictly better than the pre-fix behavior (broken Cmd+Shift+Z in sheets), but worse UX than Apple-grade apps where Cut / Copy / Delete grey out when no selection exists, and Undo / Redo grey out when no undo history exists.

Proposed fix: implement - (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)anItem on MacApplicationDelegate (toolkit/xre/MacApplicationDelegate.mm). For the seven standard Edit selectors, look up the corresponding Gecko XUL command (cmd_cut / cmd_paste / cmd_undo / etc.) in the current chrome document and return !command->GetBoolAttr(nsGkAtoms::disabled). When the chain reaches the delegate in the Gecko-focused case, Gecko's command state drives the menu enabled state; the sheet-focused case never reaches the delegate (the sheet's NSText answers first) so this doesn't regress that path.

Implementation notes:

• Selector → command ID is a 7-entry static table (cut: → cmd_cut, copy: → cmd_copy, etc.).
• NSWindow → Gecko Document bridge: reuse nsMenuBarX::sLastGeckoMenuBarPainted (or a similar class-static reference to the "current" Gecko menu bar) which already owns an Element/Document.
• Lookup: doc->GetElementById(cmdId) → cmdElement->GetBoolAttr(nsGkAtoms::disabled).

Comment 1

[Stephen A Pohl [:spohl]]

Investigation update.

I tried the approach described in the original proposal -- implementing - (BOOL)validateUserInterfaceItem: on MacApplicationDelegate, then alternatively on ChildView, looking up the matching XUL cmd_X element's disabled attribute via the menu bar's document. Neither version's validator method was ever called.

The reason is in widget/cocoa/nsMenuX.mm:973:

GeckoNSMenu* nsMenuX::CreateMenuWithGeckoString(...) {
  ...
  GeckoNSMenu* myMenu = [[GeckoNSMenu alloc] initWithTitle:title];
  ...
  // We don't want this menu to auto-enable menu items because then Cocoa
  // overrides our decisions and things get incorrectly enabled/disabled.
  myMenu.autoenablesItems = NO;
  ...
}

Every Gecko-built menu has autoenablesItems = NO. With auto-enabling off, AppKit does not run validateUserInterfaceItem: on the responder chain at all -- it just reads each NSMenuItem's explicit .enabled property and shows the item accordingly. Since bug 2040851 sets the standard Edit items to .enabled = YES (so performKeyEquivalent: won't swallow disabled shortcuts), AppKit reads that as "enabled" and never asks anyone whether the item is actionable.

The historical reason for autoenablesItems = NO is exactly the problem we are trying to invert: Gecko has its own command system (via XUL <command> disabled attribute) and the original choice was to disable AppKit's auto-validation so that Gecko's decision can't be overridden.

To actually honor Gecko's command state for the standard Edit menu items via AppKit, all three of the following are required:

1. Flip autoenablesItems = YES on at least the menu(s) containing standard Edit items (preferably globally to avoid per-menu detection).
2. Implement validateUserInterfaceItem: on NativeMenuItemTarget (the target for all Gecko-dispatched items wired with action = menuItemHit:). Without this, AppKit will see those items as "enabled because the target responds to the action" and override XUL's disabled state -- regressing the existing UX for every Gecko menu item.
3. Implement validateUserInterfaceItem: on ChildView (the standard-selector path) so the seven Edit selectors with target = nil get their enabled state from Gecko's command system via the responder chain.

Together that's an architectural change touching three classes (nsMenuX, NativeMenuItemTarget, ChildView) plus a real regression-risk surface, since turning on auto-enable for every Gecko menu and routing every item through new validation logic touches every menu in Firefox. This is too large to fold into the immediate Edit-menu work (bugs 1154697, 2040844, 2040851), and is most naturally coordinated with bug 1989226 since both bugs touch the same nsMenuX / NSMenu surface.

The shipping state without a patch for this bug is: Cut / Copy / Delete / Undo / Redo appear permanently enabled in the menu (with action-click no-ops when nothing is actionable), which I believe is better than the pre-2040851 state (broken Cmd+Shift+Z inside native sheets) and I believe is an acceptable interim while we figure out the broader fix.