Bug 1508144 - Ignore clicks on non-clickable menu items r=mixedpuppy
authorRob Wu <rob@robwu.nl>
Wed, 28 Nov 2018 15:09:26 +0000
changeset 504963 164213f935e1dfc919dd0f1fbb78cc7cdea98679
parent 504962 9d680582489f0139227b36dfd908c6f8ac154d4b
child 504964 a823c2262dbf17325c965a6fdc66f0366de2e249
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1508144, 1469148
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1508144 - Ignore clicks on non-clickable menu items r=mixedpuppy Bug 1469148 added support for detecting which mouse button was used, by synthetizing "command" events when a "click" event was captured. The implementation did not account for unclickable menu items, such as items that act as the parent of a submenu (see bug report), separators and disabled menu items. This patch adds the necessary checks and regression tests for these scenarios to make sure that such clicks are ignored, as expected. Differential Revision: https://phabricator.services.mozilla.com/D13084
browser/components/extensions/parent/ext-menus.js
browser/components/extensions/test/browser/browser_ext_menus_capture_secondary_click.js
--- a/browser/components/extensions/parent/ext-menus.js
+++ b/browser/components/extensions/parent/ext-menus.js
@@ -382,17 +382,21 @@ var gMenuBuilder = {
         let win = event.target.ownerGlobal;
         actionFor(item.extension).triggerAction(win);
       }
 
       item.extension.emit("webext-menu-menuitem-click", info, contextData.tab);
     }, {once: true});
 
     element.addEventListener("click", event => { // eslint-disable-line mozilla/balanced-listeners
-      if (event.target !== event.currentTarget) {
+      if (event.target !== event.currentTarget ||
+          // Ignore menu items that are usually not clickeable,
+          // such as separators and parents of submenus and disabled items.
+          element.localName !== "menuitem" ||
+          element.disabled) {
         return;
       }
 
       button = event.button;
       if (event.button) {
         element.doCommand();
         contextData.menu.hidePopup();
       }
--- a/browser/components/extensions/test/browser/browser_ext_menus_capture_secondary_click.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus_capture_secondary_click.js
@@ -30,8 +30,84 @@ add_task(async function test_buttons() {
     await closeExtensionContextMenu(items[0], {button: i});
     const info = await extension.awaitMessage("click");
     is(info.button, i, `Button value should be ${i}`);
   }
 
   BrowserTestUtils.removeTab(tab);
   await extension.unload();
 });
+
+add_task(async function test_submenu() {
+  function background() {
+    browser.menus.onClicked.addListener(info => {
+      browser.test.assertEq("child", info.menuItemId, "expected menu item");
+      browser.test.sendMessage("clicked_button", info.button);
+    });
+    browser.menus.create({
+      id: "parent",
+      title: "parent",
+    });
+    browser.menus.create({
+      id: "child",
+      parentId: "parent",
+      title: "child",
+    }, () => browser.test.sendMessage("ready"));
+  }
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["menus"],
+    },
+    background,
+  });
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE);
+
+  await extension.startup();
+  await extension.awaitMessage("ready");
+
+  for (let button of [0, 1, 2]) {
+    const menu = await openContextMenu();
+    const parentItem = menu.getElementsByAttribute("label", "parent")[0];
+    const submenu = await openSubmenu(parentItem);
+    const childItem = submenu.firstElementChild;
+    // This should not trigger a click event.
+    await EventUtils.synthesizeMouseAtCenter(parentItem, {button});
+    await closeExtensionContextMenu(childItem, {button});
+    is(await extension.awaitMessage("clicked_button"), button, "Expected button");
+  }
+
+  BrowserTestUtils.removeTab(tab);
+  await extension.unload();
+});
+
+add_task(async function test_disabled_item() {
+  function background() {
+    browser.menus.onHidden.addListener(() => browser.test.sendMessage("onHidden"));
+    browser.menus.create({
+      title: "disabled_item",
+      enabled: false,
+      onclick(info) {
+        browser.test.fail(`Unexpected click on disabled_item, button=${info.button}`);
+      },
+    }, () => browser.test.sendMessage("ready"));
+  }
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["menus"],
+    },
+    background,
+  });
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE);
+
+  await extension.startup();
+  await extension.awaitMessage("ready");
+
+  for (let button of [0, 1, 2]) {
+    const menu = await openContextMenu();
+    const items = menu.getElementsByAttribute("label", "disabled_item");
+    await EventUtils.synthesizeMouseAtCenter(items[0], {button});
+    await closeContextMenu();
+    await extension.awaitMessage("onHidden");
+  }
+
+  BrowserTestUtils.removeTab(tab);
+  await extension.unload();
+});