Bug 1657166 - Move block image context menu items to separate entry and cleanup mail context menu handling. r=IanN
authorFrank-Rainer Grahl <frgrahl@gmx.net>
Fri, 31 Jul 2020 17:16:30 +0200
changeset 30498 726dd53ea6219d4b096969e744202c8404217c95
parent 30497 670e4d48c646b7fbc591d3a290a3db66efe8c412
child 30499 3d2da801b8d7795ec3fdf2dceab4fa6181322997
push id17905
push userfrgrahl@gmx.net
push dateSat, 29 Aug 2020 21:30:59 +0000
treeherdercomm-central@a34803ceafa1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersIanN
bugs1657166
Bug 1657166 - Move block image context menu items to separate entry and cleanup mail context menu handling. r=IanN
suite/base/content/contentAreaContextOverlay.xul
suite/base/content/nsContextMenu.js
suite/mailnews/content/mailContextMenus.js
suite/mailnews/content/mailWindowOverlay.xul
--- a/suite/base/content/contentAreaContextOverlay.xul
+++ b/suite/base/content/contentAreaContextOverlay.xul
@@ -172,20 +172,16 @@
                 label="&reloadImageCmd.label;"
                 accesskey="&reloadImageCmd.accesskey;"
                 oncommand="gContextMenu.reloadImage();"/>
       <menuitem id="context-viewimage"
                 label="&viewImageCmd.label;"
                 accesskey="&viewImageCmd.accesskey;"
                 oncommand="gContextMenu.viewMedia(event);"
                 onclick="checkForMiddleClick(this, event);"/>
-      <menuitem id="context-blockimage"
-                oncommand="gContextMenu.toggleImageBlocking(true);"/>
-      <menuitem id="context-unblockimage"
-                oncommand="gContextMenu.toggleImageBlocking(false);"/>
       <menuitem id="context-copyimage"
                 label="&copyImageCmd.label;"
                 accesskey="&copyImageCmd.accesskey;"
                 command="cmd_copyImage"/>
       <menuitem id="context-viewvideo"
                 label="&viewVideoCmd.label;"
                 accesskey="&viewVideoCmd.accesskey;"
                 oncommand="gContextMenu.viewMedia(event);"
@@ -194,16 +190,21 @@
                 label="&copyVideoURLCmd.label;"
                 accesskey="&copyVideoURLCmd.accesskey;"
                 oncommand="gContextMenu.copyMediaLocation();"/>
       <menuitem id="context-copyaudiourl"
                 label="&copyAudioURLCmd.label;"
                 accesskey="&copyAudioURLCmd.accesskey;"
                 oncommand="gContextMenu.copyMediaLocation();"/>
       <menuseparator id="context-sep-copyimage"/>
+      <menuitem id="context-blockimage"
+                oncommand="gContextMenu.toggleImageBlocking(true);"/>
+      <menuitem id="context-unblockimage"
+                oncommand="gContextMenu.toggleImageBlocking(false);"/>
+      <menuseparator id="context-sep-blockimage"/>
       <menuitem id="context-saveimage"
                 valueSaveAs="&saveImageAsCmd.label;"
                 valueSave="&saveImageCmd.label;"
                 accesskey="&saveImageCmd.accesskey;"
                 oncommand="gContextMenu.saveMedia();"/>
       <menuitem id="context-setDesktopBackground"
                 label="&setDesktopBackgroundCmd.label;"
                 accesskey="&setDesktopBackgroundCmd.accesskey;"
--- a/suite/base/content/nsContextMenu.js
+++ b/suite/base/content/nsContextMenu.js
@@ -295,16 +295,17 @@ nsContextMenu.prototype = {
     this.showItem("context-sep-viewbgimage", showView && !this.inSyntheticDoc);
     this.setItemAttr("context-viewbgimage", "disabled", this.hasBGImage ? null : "true");
 
     this.showItem("context-viewimageinfo", this.onImage);
 
     // Hide Block and Unblock menuitems.
     this.showItem("context-blockimage", false);
     this.showItem("context-unblockimage", false);
+    this.showItem("context-sep-blockimage", false);
 
     // Block image depends on whether an image was clicked on.
     if (this.onImage) {
       var uri = Services.io.newURI(this.mediaURL);
       if (uri instanceof Ci.nsIURL && uri.host) {
         var serverLabel = uri.host;
         // Limit length to max 15 characters.
         serverLabel = serverLabel.replace(/^www\./i, "");
@@ -319,16 +320,17 @@ nsContextMenu.prototype = {
           attr = "unblockImage";
         }
         const bundle = document.getElementById("contentAreaCommandsBundle");
         this.setItemAttr(id, "label",
                          bundle.getFormattedString(attr, [serverLabel]));
         this.setItemAttr(id, "accesskey",
                          bundle.getString(attr + ".accesskey"));
         this.showItem(id, true);
+        this.showItem("context-sep-blockimage", true);
       }
     }
   },
 
   initMiscItems: function() {
     // Use "Bookmark This Link" if on a link.
     this.showItem("context-bookmarkpage",
                   !(this.isContentSelected || this.onTextInput ||
@@ -1661,23 +1663,27 @@ nsContextMenu.prototype = {
 
   isTargetATextBox: function(aNode) {
     if (aNode instanceof HTMLInputElement)
       return aNode.mozIsTextField(false) && this.isTextBoxEnabled(aNode);
 
     return aNode instanceof HTMLTextAreaElement && this.isTextBoxEnabled(aNode);
   },
 
-  // Determines whether or not the separator with the specified ID should be
-  // shown or not by determining if there are any non-hidden items between it
-  // and the previous separator.
+  /**
+   * 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
+   */
   shouldShowSeparator: function(aSeparatorID) {
-    var separator = document.getElementById(aSeparatorID);
+    let separator = document.getElementById(aSeparatorID);
     if (separator) {
-      var sibling = separator.previousSibling;
+      let sibling = separator.previousSibling;
       while (sibling && sibling.localName != "menuseparator") {
         if (sibling.getAttribute("hidden") != "true")
           return true;
         sibling = sibling.previousSibling;
       }
     }
     return false;
   },
--- a/suite/mailnews/content/mailContextMenus.js
+++ b/suite/mailnews/content/mailContextMenus.js
@@ -180,31 +180,74 @@ function FillMailContextMenu(aTarget, aE
   // 'Delete Message' menu item.
   goUpdateCommand('cmd_delete');
 
   ShowMenuItem("context-addemail", gContextMenu.onMailtoLink);
   ShowMenuItem("context-composeemailto", gContextMenu.onMailtoLink);
   ShowMenuItem("context-createfilterfrom", gContextMenu.onMailtoLink);
 
   // Figure out separators.
-  ShowSeparator("mailContext-sep-open");
-  ShowSeparator("mailContext-sep-edit");
-  ShowSeparator("mailContext-sep-link");
-  ShowSeparator("mailContext-sep-image");
-  ShowSeparator("mailContext-sep-copy");
-  ShowSeparator("mailContext-sep-print");
-  ShowSeparator("mailContext-sep-tags");
-  ShowSeparator("mailContext-sep-mark");
-  ShowSeparator("mailContext-sep-move");
+  initSeparators();
+
+  return true;
+}
+
+/**
+ * Hide separators with no active menu items.
+ *
+ */
+function initSeparators() {
+  const mailContextSeparators = [
+    "mailContext-sep-link", "mailContext-sep-open",
+    "mailContext-sep-tags", "mailContext-sep-mark",
+    "mailContext-sep-move", "mailContext-sep-print",
+    "mailContext-sep-edit", "mailContext-sep-image",
+    "mailContext-sep-blockimage", "mailContext-sep-copy",
+  ];
+
+  mailContextSeparators.forEach(hideIfAppropriate);
 
   // If we are on a link, go ahead and hide this separator.
-  if (gContextMenu.onLink)
+  if (gContextMenu.onLink) {
     ShowMenuItem("mailContext-sep-copy", false);
+  }
 
-  return true;
+  checkLastSeparator(document.getElementById("mailContext"));
+}
+
+/**
+ * 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.
+ */
+function hideIfAppropriate(aSeparatorID) {
+  ShowMenuItem(aSeparatorID, gContextMenu.shouldShowSeparator(aSeparatorID));
+}
+
+/**
+ * Ensures that there isn't a separator shown at the bottom of the menu.
+ *
+ * @param aPopup  The menu to check.
+ */
+function checkLastSeparator(aPopup) {
+  let sibling = aPopup.lastChild;
+  while (sibling) {
+    if (sibling.getAttribute("hidden") != "true") {
+      if (sibling.localName == "menuseparator") {
+        // If we got here then the item is a menuseparator and everything
+        // below it hidden.
+        sibling.setAttribute("hidden", true);
+        return;
+      }
+      else
+        return;
+    }
+    sibling = sibling.previousSibling;
+  }
 }
 
 function FolderPaneOnPopupHiding()
 {
   RestoreSelectionWithoutContentLoad(GetFolderTree());
 }
 
 function FillFolderPaneContextMenu()
@@ -402,44 +445,16 @@ function SetMenuItemLabel(id, label)
 
 function SetMenuItemAccessKey(id, accessKey)
 {
   var item = document.getElementById(id);
   if(item)
     item.setAttribute('accesskey', accessKey);
 }
 
-function SiblingHidden(aSibling, aNext)
-{
-  var siblingID;
-  while (aSibling)
-  {
-    siblingID = aSibling.id;
-    // For some reason, context-blockimage and context-unblockimage are not
-    // hidden on the very first time the context menu is invoked. They are only
-    // hidden on subsequent triggers of the context menu. Since we're not
-    // using these two menuitems in mailnews, we can ignore them if encountered.
-    if (!aSibling.hidden && (siblingID != "context-blockimage") &&
-        (siblingID != "context-unblockimage"))
-      return aSibling.localName == "menuseparator";
-    aSibling = aNext ? aSibling.nextSibling : aSibling.previousSibling;
-  }
-  return true;
-}
-
-function ShowSeparator(aSeparatorID)
-{
-  var separator = document.getElementById(aSeparatorID);
-  // Check to see if there are visible siblings before the next and
-  // previous separators.
-  var hidden = SiblingHidden(separator.nextSibling, true) ||
-               SiblingHidden(separator.previousSibling, false);
-  ShowMenuItem(aSeparatorID, !hidden);
-}
-
 // message pane context menu helper methods
 function AddContact(aEmailAddressNode)
 {
   if (aEmailAddressNode)
     AddEmailToAddressBook(aEmailAddressNode.getAttribute("emailAddress"),
                           aEmailAddressNode.getAttribute("displayName"));
 }
 
--- a/suite/mailnews/content/mailWindowOverlay.xul
+++ b/suite/mailnews/content/mailWindowOverlay.xul
@@ -697,25 +697,26 @@
     <menuitem id="context-copyimage"
               label="&copyImageCmd.label;"
               accesskey="&copyImageCmd.accesskey;"
               command="cmd_copyImage"/>
     <menuitem id="context-viewimage"
               label="&viewImageCmd.label;"
               accesskey="&viewImageCmd.accesskey;"
               oncommand="gContextMenu.viewMedia();"/>
-    <menuitem id="context-blockimage"
-              oncommand="gContextMenu.toggleImageBlocking(true);"/>
-    <menuitem id="context-unblockimage"
-              oncommand="gContextMenu.toggleImageBlocking(false);"/>
     <menuitem id="context-addemail"
               label="&AddToAddressBook.label;"
               accesskey="&AddToAddressBook.accesskey;"
               oncommand="AddEmailToAddressBook(gContextMenu.getEmail(), gContextMenu.linkText());"/>
     <menuseparator id="mailContext-sep-image"/>
+    <menuitem id="context-blockimage"
+              oncommand="gContextMenu.toggleImageBlocking(true);"/>
+    <menuitem id="context-unblockimage"
+              oncommand="gContextMenu.toggleImageBlocking(false);"/>
+    <menuseparator id="mailContext-sep-blockimage"/>
     <menuitem id="context-composeemailto"
               label="&SendMailTo.label;"
               accesskey="&SendMailTo.accesskey;"
               oncommand="SendMailTo(gContextMenu.getEmail(), event);"/>
     <menuitem id="context-createfilterfrom"
               label="&CreateFilterFrom.label;"
               accesskey="&CreateFilterFrom.accesskey;"
               oncommand="CreateFilterFromMail(gContextMenu.getEmail());"/>