Bug 1508144 - Ignore clicks on non-clickable menu items. r=mixedpuppy, a=jcristau
authorRob Wu <rob@robwu.nl>
Wed, 28 Nov 2018 15:09:26 +0000
changeset 501429 3be25c15b7e9e3326826739bdc4f24ba94e8578b
parent 501428 24a6a9b747b34914d8dcd6ae714ca37fe5d56739
child 501430 f4fe9800063b5110cb59f2eafdc1ffc45feb5a60
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy, jcristau
bugs1508144, 1469148
milestone64.0
Bug 1508144 - Ignore clicks on non-clickable menu items. r=mixedpuppy, a=jcristau 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();
+});