Bug 1817518 - De-select closed menuitems on mouseout. r=mstange

I knew this sounded familiar, see bug 1811466.

The patch is a one-liner, the rest is a test. I'd ask smaug for review
but he's on PTO so... Markus, maybe you can take a look?

Differential Revision: https://phabricator.services.mozilla.com/D170269

dom/xul/XULButtonElement.cpp

@@ -444,16 +444,18 @@ void XULButtonElement::PostHandleEventForMenus(
             // If we're open we never deselect. PopupClosed will do as needed.
             return false;
           }
-          auto* menubar = XULMenuBarElement::FromNode(*parent);
-          if (!menubar) {
-            // Don't de-select when not in the menubar.
-            // NOTE(emilio): Behavior from before bug 1811466 is equivalent to
-            // returning true here, consider flipping this.
+          if (auto* menubar = XULMenuBarElement::FromNode(*parent)) {
+            // De-select when exiting a menubar item, if the menubar wasn't
+            // activated by keyboard.
+            return !menubar->IsActiveByKeyboard();
+          }
+          if (IsOnMenuList()) {
+            // Don't de-select if on a menu-list. That matches Chromium and our
+            // historical Windows behavior, see bug 1197913.
             return false;
           }
-          // De-select when exiting a menubar item, if the menubar wasn't
-          // activated by keyboard.
-          return !menubar->IsActiveByKeyboard();
+          // De-select elsewhere.
+          return true;
         }();
 
         if (shouldDeactivate) {

toolkit/content/tests/chrome/chrome.ini

@@ -132,6 +132,7 @@ support-files =
 support-files = window_navigate_persist.html
 [test_menu.xhtml]
 [test_menu_activateitem.xhtml]
+[test_menu_mouse_menuactive.xhtml]
 [test_menu_withcapture.xhtml]
 [test_menu_hide.xhtml]
 [test_menuchecks.xhtml]

toolkit/content/tests/chrome/test_menu_activateitem.xhtml

@@ -54,7 +54,7 @@ function checkActivate(desc, menu, item, expectedResult)
 async function checkActivateItems(desc, openFn, expectedResults)
 {
   // Iterate over each menu/submenu and try activating the item from that menu.
-  // This should only pass when the item is a descedant of that menu and the
+  // This should only pass when the item is a descendant of that menu and the
   // menu is open.
   let menus = [ "contextmenu", "submenu", "innersubmenu" ];
   let items = [ "item1", "item2", "item3", "item4" ];

toolkit/content/tests/chrome/test_menu_mouse_menuactive.xhtml

@@ -0,0 +1,89 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+
+<window title="Menu Activation Test"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
+
+  <label id="label" value="label" context="contextmenu"/>
+  <menupopup id="contextmenu">
+    <menuitem id="item1" label="Item 1"/>
+    <menuitem id="item2" label="Item 2"/>
+  </menupopup>
+  <script><![CDATA[
+
+function waitForEvent(subject, eventName) {
+  return new Promise(resolve => {
+    subject.addEventListener(eventName, function listener(event) {
+      if (event.target == subject) {
+        subject.removeEventListener(eventName, listener);
+        resolve();
+      }
+    });
+  });
+}
+
+const menu = document.getElementById("contextmenu");
+const label = document.getElementById("label");
+const item1 = document.getElementById("item1");
+const item2 = document.getElementById("item2");
+
+function openContextMenu() {
+  // Open the first level of the context menu
+  let promise = waitForEvent(menu, "popupshown");
+  synthesizeMouseAtCenter(label, {
+    type: "contextmenu",
+    button: 2,
+  });
+  return promise;
+}
+
+function isActive(item) {
+  return item.hasAttribute("_moz-menuactive");
+}
+
+function activateItem(item) {
+  info(`Activating ${item.id}`);
+  ok(!isActive(item), "Shouldn't be already active");
+  let promise = waitForEvent(item, "DOMMenuItemActive");
+  synthesizeMouseAtCenter(item, { type: "mousemove" });
+  return promise;
+}
+
+add_task(async function() {
+  // Disable macOS native context menus, since we can't control activation of those here.
+  await SpecialPowers.pushPrefEnv({ set: [["widget.macos.native-context-menus", false]] });
+  info(`Opening context-menu`);
+  await openContextMenu();
+  is(menu.state, "open", "Menu should be open");
+
+  await activateItem(item1);
+  ok(isActive(item1), "item1 should be active");
+  ok(!isActive(item2), "item2 should be inactive");
+
+  await activateItem(item2);
+  ok(isActive(item2), "item2 should be active");
+  ok(!isActive(item1), "item1 should be inactive");
+
+  await activateItem(item1);
+  ok(isActive(item1), "item1 should be active");
+  ok(!isActive(item2), "item2 should be inactive");
+
+  info(`Leaving context-menu`);
+
+  let deactivated = waitForEvent(item1, "DOMMenuItemInactive");
+  synthesizeMouse(label, 0, 0, { type: "mousemove" });
+
+  await deactivated;
+  ok(!isActive(item1), "item1 should be inactive");
+  ok(!isActive(item2), "item2 should be inactive");
+
+  is(menu.state, "open", "Menu should still be open");
+  menu.hidePopup();
+});
+
+  ]]></script>
+</window>