Bug 1215376 - Refactor gMenuBuilder.build draft
authorRob Wu <rob@robwu.nl>
Tue, 12 Sep 2017 02:34:41 +0200
changeset 716032 46be6fc1990283364c624a3d3240f207cef84261
parent 716031 d9a1b8cc345e2295eee62701a2ecc51cf861a3c1
child 716033 ba817b5de4ada2f4bca8668ef7c79b1c18bfba73
push id94307
push userbmo:rob@robwu.nl
push dateFri, 05 Jan 2018 00:24:53 +0000
bugs1215376
milestone59.0a1
Bug 1215376 - Refactor gMenuBuilder.build Splot gMenuBuilder.build in three separate functions because the individual functionality is needed later. This is just moving some code around, nothing interesting to see. MozReview-Commit-ID: Lo6QmLQBtO5
browser/components/extensions/ext-menus.js
--- a/browser/components/extensions/ext-menus.js
+++ b/browser/components/extensions/ext-menus.js
@@ -42,44 +42,24 @@ var gNextRadioGroupID = 0;
 var gMaxLabelLength = 64;
 
 var gMenuBuilder = {
   // When a new menu is opened, this function is called and
   // we populate the |xulMenu| with all the items from extensions
   // to be displayed. We always clear all the items again when
   // popuphidden fires.
   build(contextData) {
-    let firstItem = true;
     let xulMenu = contextData.menu;
     xulMenu.addEventListener("popuphidden", this);
     this.xulMenu = xulMenu;
     for (let [, root] of gRootItems) {
-      let rootElement = this.buildElementWithChildren(root, contextData);
-      if (!rootElement.firstChild || !rootElement.firstChild.childNodes.length) {
-        // If the root has no visible children, there is no reason to show
-        // the root menu item itself either.
-        continue;
+      let rootElement = this.createTopLevelElement(root, contextData);
+      if (rootElement) {
+        this.appendTopLevelElement(rootElement);
       }
-      rootElement.setAttribute("ext-type", "top-level-menu");
-      rootElement = this.removeTopLevelMenuIfNeeded(rootElement);
-
-      // Display the extension icon on the root element.
-      if (root.extension.manifest.icons) {
-        this.setMenuItemIcon(rootElement, root.extension, contextData, root.extension.manifest.icons);
-      }
-
-      if (firstItem) {
-        firstItem = false;
-        const separator = xulMenu.ownerDocument.createElement("menuseparator");
-        this.itemsToCleanUp.add(separator);
-        xulMenu.append(separator);
-      }
-
-      xulMenu.appendChild(rootElement);
-      this.itemsToCleanUp.add(rootElement);
     }
     this.afterBuildingMenu(contextData);
   },
 
   // Builds a context menu for browserAction and pageAction buttons.
   buildActionContextMenu(contextData) {
     const {menu} = contextData;
 
@@ -134,16 +114,44 @@ var gMenuBuilder = {
 
       if (child.enabledForContext(contextData)) {
         children.push(this.buildElementWithChildren(child, contextData));
       }
     }
     return children;
   },
 
+  createTopLevelElement(root, contextData) {
+    let rootElement = this.buildElementWithChildren(root, contextData);
+    if (!rootElement.firstChild || !rootElement.firstChild.childNodes.length) {
+      // If the root has no visible children, there is no reason to show
+      // the root menu item itself either.
+      return null;
+    }
+    rootElement.setAttribute("ext-type", "top-level-menu");
+    rootElement = this.removeTopLevelMenuIfNeeded(rootElement);
+
+    // Display the extension icon on the root element.
+    if (root.extension.manifest.icons) {
+      this.setMenuItemIcon(rootElement, root.extension, contextData, root.extension.manifest.icons);
+    }
+    return rootElement;
+  },
+
+  appendTopLevelElement(rootElement) {
+    if (this.itemsToCleanUp.size === 0) {
+      const separator = this.xulMenu.ownerDocument.createElement("menuseparator");
+      this.itemsToCleanUp.add(separator);
+      this.xulMenu.append(separator);
+    }
+
+    this.xulMenu.appendChild(rootElement);
+    this.itemsToCleanUp.add(rootElement);
+  },
+
   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