Bug 1351418 - Put single checkbox items into submenu on linux r?mixedpuppy,jkt draft
authorMatthew Wein <mwein@mozilla.com>
Mon, 15 May 2017 13:20:41 -0400
changeset 580823 70b74d27a66763031e546f8174a5a6225622268c
parent 580822 3801e9bd76ad91b5754faacd3fec9ad4bc3fc782
child 629391 23169de6c7783aaaefa8c6b994bbb4263fd8bd59
push id59668
push usermwein@mozilla.com
push dateFri, 19 May 2017 00:12:44 +0000
reviewersmixedpuppy, jkt
bugs1351418
milestone55.0a1
Bug 1351418 - Put single checkbox items into submenu on linux r?mixedpuppy,jkt MozReview-Commit-ID: 9swHS0yEoDg
browser/components/extensions/ext-contextMenus.js
browser/components/extensions/test/browser/browser_ext_contextMenus_checkboxes.js
--- a/browser/components/extensions/ext-contextMenus.js
+++ b/browser/components/extensions/ext-contextMenus.js
@@ -142,16 +142,23 @@ var gMenuBuilder = {
   },
 
   removeTopLevelMenuIfNeeded(element) {
     // If there is only one visible top level element we don't need the
     // root menu element for the extension.
     let menuPopup = element.firstChild;
     if (menuPopup && menuPopup.childNodes.length == 1) {
       let onlyChild = menuPopup.firstChild;
+
+      // Keep single checkbox items in the submenu on Linux since
+      // the extension icon overlaps the checkbox otherwise.
+      if (AppConstants.platform === "linux" && onlyChild.getAttribute("type") === "checkbox") {
+        return element;
+      }
+
       onlyChild.remove();
       return onlyChild;
     }
 
     return element;
   },
 
   buildSingleElement(item, contextData) {
--- a/browser/components/extensions/test/browser/browser_ext_contextMenus_checkboxes.js
+++ b/browser/components/extensions/test/browser/browser_ext_contextMenus_checkboxes.js
@@ -19,36 +19,64 @@ add_task(async function() {
         browser.test.sendMessage("contextmenus-click", info);
       });
 
       browser.contextMenus.create({
         title: "Checkbox",
         type: "checkbox",
       });
 
-      browser.contextMenus.create({
-        type: "separator",
-      });
+      browser.test.sendMessage("single-contextmenu-item-added");
+
+      browser.test.onMessage.addListener(msg => {
+        if (msg !== "add-additional-menu-items") {
+          return;
+        }
+
+        browser.contextMenus.create({
+          type: "separator",
+        });
 
-      browser.contextMenus.create({
-        title: "Checkbox",
-        type: "checkbox",
-        checked: true,
-      });
+        browser.contextMenus.create({
+          title: "Checkbox",
+          type: "checkbox",
+          checked: true,
+        });
 
-      browser.contextMenus.create({
-        title: "Checkbox",
-        type: "checkbox",
+        browser.contextMenus.create({
+          title: "Checkbox",
+          type: "checkbox",
+        });
+
+        browser.test.notifyPass("contextmenus-checkboxes");
       });
-
-      browser.test.notifyPass("contextmenus-checkboxes");
     },
   });
 
   await extension.startup();
+
+  await extension.awaitMessage("single-contextmenu-item-added");
+
+  async function testSingleCheckboxItem() {
+    let extensionMenuRoot = await openExtensionContextMenu();
+
+    // On Linux, the single menu item should be contained in a submenu.
+    if (AppConstants.platform === "linux") {
+      let items = extensionMenuRoot.getElementsByAttribute("type", "checkbox");
+      is(items.length, 1, "single checkbox should be in the submenu on Linux");
+      await closeContextMenu();
+    } else {
+      is(extensionMenuRoot, null, "there should be no submenu for a single checkbox item");
+      await closeContextMenu();
+    }
+  }
+
+  await testSingleCheckboxItem();
+
+  extension.sendMessage("add-additional-menu-items");
   await extension.awaitFinish("contextmenus-checkboxes");
 
   function confirmCheckboxStates(extensionMenuRoot, expectedStates) {
     let checkboxItems = extensionMenuRoot.getElementsByAttribute("type", "checkbox");
 
     is(checkboxItems.length, 3, "there should be 3 checkbox items in the context menu");
 
     is(checkboxItems[0].hasAttribute("checked"), expectedStates[0], `checkbox item 1 has state (checked=${expectedStates[0]})`);