Bug 1787130 - Fix double separator for webExt menu entries. r=aleca
authorJohn Bieling <john@thunderbird.net>
Tue, 13 Sep 2022 22:20:13 +0000
changeset 36726 45d0d89ca3fc212f3f1b0a3069a29b2f05cb36dd
parent 36725 11150dcde6a586759c4d41dbd401823de65e20af
child 36727 e9e43627b8c4741f6d82b573f389a7901f5159d8
push id20282
push user[email protected]
push dateTue, 13 Sep 2022 22:22:21 +0000
treeherdercomm-central@45d0d89ca3fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaleca
bugs1787130
Bug 1787130 - Fix double separator for webExt menu entries. r=aleca Differential Revision: https://phabricator.services.mozilla.com/D157187
mail/base/content/nsContextMenu.js
mail/components/extensions/parent/ext-menus.js
--- a/mail/base/content/nsContextMenu.js
+++ b/mail/base/content/nsContextMenu.js
@@ -729,42 +729,38 @@ class nsContextMenu {
 
     this.showItem(
       "mailContext-reportPhishingURL",
       !this.inThreadPane && this.onLink && !this.onMailtoLink
     );
 
     this.setSingleSelection("mailContext-calendar-convert-menu");
   }
-  /* eslint-enable complexity */
   initSeparators() {
-    const mailContextSeparators = [
-      "mailContext-sep-open-browser",
-      "mailContext-sep-link",
-      "mailContext-sep-open",
-      "mailContext-sep-open2",
-      "mailContext-sep-reply",
-      "paneContext-afterMove",
-      "mailContext-sep-afterTagAddNew",
-      "mailContext-sep-afterTagRemoveAll",
-      "mailContext-sep-afterMarkAllRead",
-      "mailContext-sep-afterMarkFlagged",
-      "mailContext-sep-afterMarkMenu",
-      "mailContext-afterWatchThread",
-      "mailContext-sep-edit",
-      "mailContext-sep-editTemplate",
-      "mailContext-sep-copy",
-      "mailContext-sep-reportPhishing",
-      "mailContext-sep-undo",
-      "mailContext-sep-clipboard",
-      "mailContext-spell-suggestions-separator",
-      "mailContext-spell-separator",
-    ];
-    mailContextSeparators.forEach(this.hideIfAppropriate, this);
-
+    let separators = Array.from(
+      this.xulMenu.querySelectorAll(":scope > menuseparator")
+    );
+    let lastShownSeparator = null;
+    for (let separator of separators) {
+      let shouldShow = this.shouldShowSeparator(separator);
+      if (
+        !shouldShow &&
+        lastShownSeparator &&
+        separator.classList.contains("webextension-group-separator")
+      ) {
+        // The separator for the WebExtension elements group must be shown, hide
+        // the last shown menu separator instead.
+        lastShownSeparator.hidden = true;
+        shouldShow = true;
+      }
+      if (shouldShow) {
+        lastShownSeparator = separator;
+      }
+      separator.hidden = !shouldShow;
+    }
     this.checkLastSeparator(this.xulMenu);
   }
 
   setMessageTargets() {
     if (this.browser) {
       this.inAMessage = ["imap", "mailbox", "news", "snews"].includes(
         this.browser.currentURI.scheme
       );
@@ -1066,36 +1062,24 @@ class nsContextMenu {
     if (HTMLInputElement.isInstance(aNode)) {
       return aNode.type == "text" || aNode.type == "password";
     }
 
     return HTMLTextAreaElement.isInstance(aNode);
   }
 
   /**
-   * Hide a separator based on whether there are any non-hidden items between
-   * it and the previous separator.
-   *
-   * @param aSeparatorID  The id of the separator element.
-   */
-  hideIfAppropriate(aSeparatorID) {
-    this.showItem(aSeparatorID, this.shouldShowSeparator(aSeparatorID));
-  }
-
-  /**
    * Determine whether a separator should be shown based on whether
    * there are any non-hidden items between it and the previous separator.
-   * @param  aSeparatorID
-   *         The id of the separator element
-   * @return true if the separator should be shown, false if not
+   * @param {DomElement} element - The separator element.
+   * @return {boolean} True if the separator should be shown, false if not.
    */
-  shouldShowSeparator(aSeparatorID) {
-    var separator = document.getElementById(aSeparatorID);
-    if (separator) {
-      var sibling = separator.previousElementSibling;
+  shouldShowSeparator(element) {
+    if (element) {
+      let sibling = element.previousElementSibling;
       while (sibling && sibling.localName != "menuseparator") {
         if (!sibling.hidden) {
           return true;
         }
         sibling = sibling.previousElementSibling;
       }
     }
     return false;
--- a/mail/components/extensions/parent/ext-menus.js
+++ b/mail/components/extensions/parent/ext-menus.js
@@ -109,16 +109,24 @@ var gMenuBuilder = {
       };
     }
     throw new Error(
       `Unexpected overrideContext: ${webExtContextData.overrideContext}`
     );
   },
 
   createAndInsertTopLevelElements(root, contextData, nextSibling) {
+    const newWebExtensionGroupSeparator = () => {
+      let element = this.xulMenu.ownerDocument.createXULElement(
+        "menuseparator"
+      );
+      element.classList.add("webextension-group-separator");
+      return element;
+    };
+
     let rootElements;
     if (
       contextData.onBrowserAction ||
       contextData.onComposeAction ||
       contextData.onMessageDisplayAction
     ) {
       if (contextData.extension.id !== root.extension.id) {
         return;
@@ -128,19 +136,17 @@ var gMenuBuilder = {
         contextData,
         ACTION_MENU_TOP_LEVEL_LIMIT,
         false
       );
 
       // Action menu items are prepended to the menu, followed by a separator.
       nextSibling = nextSibling || this.xulMenu.firstElementChild;
       if (rootElements.length && !this.itemsToCleanUp.has(nextSibling)) {
-        rootElements.push(
-          this.xulMenu.ownerDocument.createXULElement("menuseparator")
-        );
+        rootElements.push(newWebExtensionGroupSeparator());
       }
     } else if (contextData.webExtContextData) {
       let {
         extensionId,
         showDefaults,
         overrideContext,
       } = contextData.webExtContextData;
       if (extensionId === root.extension.id) {
@@ -153,38 +159,34 @@ var gMenuBuilder = {
         // The extension menu should be rendered at the top, but after the navigation buttons.
         nextSibling =
           nextSibling || this.xulMenu.querySelector(":scope > :first-child");
         if (
           rootElements.length &&
           showDefaults &&
           !this.itemsToCleanUp.has(nextSibling)
         ) {
-          rootElements.push(
-            this.xulMenu.ownerDocument.createXULElement("menuseparator")
-          );
+          rootElements.push(newWebExtensionGroupSeparator());
         }
       } else if (!showDefaults && !overrideContext) {
         // When the default menu items should be hidden, menu items from other
         // extensions should be hidden too.
         return;
       }
       // Fall through to show default extension menu items.
     }
     if (!rootElements) {
       rootElements = this.buildTopLevelElements(root, contextData, 1, true);
       if (
         rootElements.length &&
         !this.itemsToCleanUp.has(this.xulMenu.lastElementChild)
       ) {
         // All extension menu items are appended at the end.
         // Prepend separator if this is the first extension menu item.
-        rootElements.unshift(
-          this.xulMenu.ownerDocument.createXULElement("menuseparator")
-        );
+        rootElements.unshift(newWebExtensionGroupSeparator());
       }
     }
 
     if (!rootElements.length) {
       return;
     }
 
     if (nextSibling) {