Bug 1679883 - Hide separators in mail window better. r=frg
authorIan Neal <iann_cvs@blueyonder.co.uk>
Mon, 30 Nov 2020 16:34:37 +0000
changeset 31229 ad79488515edf3dbd423eb7153bf4565c348bc10
parent 31228 a1af995ab28e4369e417ac21cc0abdfaf12d13a3
child 31230 4c5bea8b0b13d2fdb7e98f42a530b5ca65060286
push id18261
push userfrgrahl@gmx.net
push dateWed, 09 Dec 2020 23:15:25 +0000
treeherdercomm-central@ad79488515ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfrg
bugs1679883
Bug 1679883 - Hide separators in mail window better. r=frg
suite/mailnews/content/mailContextMenus.js
suite/mailnews/content/mailWindowOverlay.xul
--- a/suite/mailnews/content/mailContextMenus.js
+++ b/suite/mailnews/content/mailContextMenus.js
@@ -66,18 +66,22 @@ function RestoreSelectionWithoutContentL
 }
 
 /**
  * Function to clear out the global nsContextMenu, and in the case when we
  * are a threadpane context menu, restore the selection so that a right-click
  * on a non-selected row doesn't move the selection.
  * @param aTarget the target of the popup event
  */
-function MailContextOnPopupHiding(aTarget)
-{
+function MailContextOnPopupHiding(aTarget, aEvent) {
+  // Don't do anything if it's a submenu's onpopuphiding that's just bubbling
+  // up to the top.
+  if (aEvent.target != aTarget)
+    return;
+
   gContextMenu.hiding();
   gContextMenu = null;
   if (InThreadPane(aTarget))
     RestoreSelectionWithoutContentLoad(GetThreadTree());
 }
 
 /**
  * Determines whether the context menu was triggered by a node that's a child
@@ -97,18 +101,21 @@ function InThreadPane(aTarget)
   return false;
 }
 
 /**
  * Function to set up the global nsContextMenu, and the mailnews overlay.
  * @param aTarget the target of the popup event
  * @return true always
  */
-function FillMailContextMenu(aTarget, aEvent)
-{
+function FillMailContextMenu(aTarget, aEvent) {
+  // If the popupshowing was for a submenu, we don't need to do anything.
+  if (aEvent.target != aTarget)
+    return true;
+
   var inThreadPane = InThreadPane(aTarget);
   gContextMenu = new nsContextMenu(aTarget);
 
   // Initialize gContextMenuContentData.
   if (aEvent)
     gContextMenu.initContentData(aEvent);
 
   // Need to call nsContextMenu's initItems to hide what is not used.
@@ -202,55 +209,48 @@ function initSeparators() {
     "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) {
-    ShowMenuItem("mailContext-sep-copy", false);
-  }
-
-  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.
+ * @param aID  The id of the separator element.
  */
-function hideIfAppropriate(aSeparatorID) {
-  ShowMenuItem(aSeparatorID, gContextMenu.shouldShowSeparator(aSeparatorID));
-}
+function hideIfAppropriate(aID) {
+  let separator = document.getElementById(aID);
 
-/**
- * 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;
+  function hasAVisibleNextSibling(aNode) {
+    let sibling = aNode.nextSibling;
+    while (sibling) {
+      if (sibling.getAttribute("hidden") != "true" &&
+          sibling.localName != "menuseparator")
+        return true;
+      sibling = sibling.nextSibling;
+    }
+    return false;
+  }
+
+  let sibling = separator.previousSibling;
   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;
+      ShowMenuItem(aID, sibling.localName != "menuseparator" &&
+                        hasAVisibleNextSibling(separator));
+      return;
     }
     sibling = sibling.previousSibling;
   }
+  ShowMenuItem(aID, false);
 }
 
 function FolderPaneOnPopupHiding()
 {
   RestoreSelectionWithoutContentLoad(GetFolderTree());
 }
 
 function FillFolderPaneContextMenu()
@@ -462,19 +462,19 @@ function FillFolderPaneContextMenu()
   ShowMenuItem("folderPaneContext-searchMessages",
                numSelected == 1 && !haveAnyVirtualFolders);
   goUpdateCommand('cmd_search');
 
   ShowMenuItem("folderPaneContext-openNewWindow", numSelected == 1);
   ShowMenuItem("folderPaneContext-openNewTab", numSelected == 1);
 
   // Hide / Show our menu separators based on the menu items we are showing.
-  ShowMenuItem("folderPaneContext-sep-edit", numSelected == 1);
-  ShowMenuItem("folderPaneContext-sep1", selectedServers.length == 0);
-  ShowMenuItem("folderPaneContext-sep4", selectedServers.length > 0);
+  hideIfAppropriate("folderPaneContext-sep1");
+  hideIfAppropriate("folderPaneContext-sep-edit");
+  hideIfAppropriate("folderPaneContext-sep4");
 
   return true;
 }
 
 function ShowMenuItem(id, showItem)
 {
   var item = document.getElementById(id);
   if(item && item.hidden != "true")
--- a/suite/mailnews/content/mailWindowOverlay.xul
+++ b/suite/mailnews/content/mailWindowOverlay.xul
@@ -499,19 +499,18 @@
     <menuseparator id="folderPaneContext-sep4"/>
     <menuitem id="folderPaneContext-settings"
               label="&folderContextSettings.label;"
               accesskey="&folderContextSettings.accesskey;"
               oncommand="gFolderTreeController.editFolder();"/>
   </menupopup>
 
   <menupopup id="mailContext"
-         onpopupshowing="return event.target != this ||
-                                FillMailContextMenu(this, event);"
-         onpopuphiding="if (event.target == this) MailContextOnPopupHiding(this);">
+         onpopupshowing="return FillMailContextMenu(this, event);"
+         onpopuphiding="MailContextOnPopupHiding(this, event);">
     <menuitem id="context-openlinkintab"
               label="&openLinkCmdInTab.label;"
               accesskey="&openLinkCmdInTab.accesskey;"
               usercontextid="0"
               oncommand="gContextMenu.openLinkInTab(event);"/>
     <menuitem id="context-openlink"
               label="&openLinkCmd.label;"
               accesskey="&openLinkCmd.accesskey;"