Bug 809066 - In the folder picker, generate the Recent and Favorites menus only if opened. r=mkmelin
authoraceman <acelists@atlas.sk>
Thu, 10 Jan 2019 19:56:17 +1300
changeset 33294 b7fe10c2833b
parent 33293 a0540daeda24
child 33295 cb009c395002
push id2368
push userclokep@gmail.com
push dateMon, 28 Jan 2019 21:12:50 +0000
treeherdercomm-beta@56d23c07d815 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin
bugs809066
Bug 809066 - In the folder picker, generate the Recent and Favorites menus only if opened. r=mkmelin
mail/test/mozmill/folder-display/test-recent-menu.js
mailnews/base/content/folderWidgets.xml
--- a/mail/test/mozmill/folder-display/test-recent-menu.js
+++ b/mail/test/mozmill/folder-display/test-recent-menu.js
@@ -42,19 +42,19 @@ var setupModule = function(module) {
 
 function test_move_message() {
   be_in_folder(folder1);
   let msgHdr = select_click_row(0);
   // This will cause the initial build of the move recent context menu,
   // which should be empty and disabled.
   right_click_on_row(0);
   let popups = mc.click_menus_in_sequence(mc.e("mailContext"),
-                                          [{id: "mailContext-moveMenu"}], true);
-  let recentMenu = popups[popups.length - 1]
-                         .querySelector('[label="Recent"]');
+                                          [{id: "mailContext-moveMenu"},
+                                           {label: "Recent"}], true);
+  let recentMenu = popups[popups.length - 2].querySelector('[label="Recent"]');
   assert_equals(recentMenu.getAttribute("disabled"), "true");
   gInitRecentMenuCount = recentMenu.itemCount;
   assert_equals(gInitRecentMenuCount, 0);
   mc.close_popup_sequence(popups);
   let array = Cc["@mozilla.org/array;1"]
                 .createInstance(Ci.nsIMutableArray);
   array.appendElement(msgHdr);
   let copyListener = {
--- a/mailnews/base/content/folderWidgets.xml
+++ b/mailnews/base/content/folderWidgets.xml
@@ -194,52 +194,54 @@
 
       <!--
          - Our listener to let us know when folders change/appear/disappear so
          - we can know to rebuild ourselves.
         -->
       <field name="_listener">
           <![CDATA[({
         _menu: this,
-        _clearMenu: function(aMenu) {
-          if (aMenu._teardown)
-            aMenu._teardown();
+        _clearMenu(menu) {
+          //xxx I'm not quite sure why this isn't always a function (bug 514445)
+          if (menu._teardown)
+            menu._teardown();
         },
         OnItemAdded: function act_add(aRDFParentItem, aItem) {
           if (!(aItem instanceof Ci.nsIMsgFolder))
             return;
           if (this._filterFunction && !this._filterFunction(aItem)) {
             return;
           }
           //xxx we can optimize this later
-          //xxx I'm not quite sure why this isn't always a function
-          if (this._menu._teardown)
-            this._menu._teardown();
+          this._clearMenu(this._menu);
         },
 
         OnItemRemoved: function act_remove(aRDFParentItem, aItem) {
           if (!(aItem instanceof Ci.nsIMsgFolder))
             return;
           if (this._filterFunction && !this._filterFunction(aItem)) {
             return;
           }
           //xxx we can optimize this later
-          if (this._menu._teardown)
-            this._menu._teardown();
+          this._clearMenu(this._menu);
         },
 
         //xxx I stole this listener list from nsMsgFolderDatasource.cpp, but
         //    someone should really document what events are fired when, so that
         //    we make sure we're updating at the right times.
         OnItemPropertyChanged: function(aItem, aProperty, aOld, aNew) {},
         OnItemIntPropertyChanged: function(aItem, aProperty, aOld, aNew) {
           if (aItem instanceof Ci.nsIMsgFolder) {
-            if (aProperty == "FolderFlag" &&
-                ((aOld & Ci.nsMsgFolderFlags.Favorite) !=
-                 (aNew & Ci.nsMsgFolderFlags.Favorite))) {
+            if (aProperty == "FolderFlag") {
+              if (this._menu.getAttribute("showFavorites") != "true" ||
+                  !this._menu._initializedSpecials.has("favorites"))
+                return;
+
+              if ((aOld & Ci.nsMsgFolderFlags.Favorite) !=
+                 (aNew & Ci.nsMsgFolderFlags.Favorite))
                setTimeout(this._clearMenu, 0, this._menu);
             }
           }
           var child = this._getChildForItem(aItem);
           if (child)
             this._menu._setCssSelectors(child._folder, child);
         },
         OnItemBoolPropertyChanged: function(aItem, aProperty, aOld, aNew) {
@@ -251,16 +253,17 @@
           var child = this._getChildForItem(aItem);
           if (child)
             this._menu._setCssSelectors(child._folder, child);
         },
         OnItemPropertyFlagChanged: function(aItem, aProperty, aOld, aNew) {},
         OnItemEvent: function(aFolder, aEvent) {
           if (aEvent == "MRMTimeChanged") {
             if (this._menu.getAttribute("showRecent") != "true" ||
+                !this._menu._initializedSpecials.has("recent") ||
                 !this._menu.firstChild || !this._menu.firstChild.firstChild)
               return;
             // if this folder is already in the recent menu, return.
             if (this._getChildForItem(aFolder,
                                       this._menu.firstChild.firstChild))
               return;
           }
           // Special casing folder renames here, since they require more work
@@ -281,35 +284,41 @@
          *
          * @param aItem  the nsIMsgFolder to check
          * @param aMenu  (optional) menu to look in, defaults to this._menu.
          * @returns      null if no child for that folder exists, otherwise the
          *               menuitem for that child
          */
         _getChildForItem: function act__itemIsChild(aItem, aMenu) {
           aMenu = aMenu || this._menu;
-          if (!aMenu || !aMenu.childNodes)
+          if (!aMenu || !aMenu.hasChildNodes())
             return null;
 
            if (!(aItem instanceof Ci.nsIMsgFolder))
              return null;
            for (let i = 0; i < aMenu.childNodes.length; i++) {
              let folder = aMenu.childNodes[i]._folder;
              if (folder && folder.URI == aItem.URI)
                return aMenu.childNodes[i];
            }
            return null;
         }
       })]]></field>
 
-      <!--
+       <!--
          - True if we have already built our menu-items and are now just
-         - listening for changes
+         - listening for changes.
+         -->
+      <field name="_initialized">false</field>
+
+      <!--
+         - A Set listing which of our special menus are already built.
+         - E.g. "recent", "favorites".
         -->
-      <field name="_initialized">false</field>
+      <field name="_initializedSpecials">new Set()</field>
 
       <!--
          - Call this if you are unsure whether the menu-items have been built,
          - but know that they need to be built now if they haven't.
         -->
       <method name="_ensureInitialized">
         <body><![CDATA[
           if (this._initialized)
@@ -368,16 +377,17 @@
           // above note on the _filters field.
           var mode = this.getAttribute("mode");
           if (mode && mode != "") {
             var filterFunction = this._filters[mode];
             folders = aFolders.filter(filterFunction);
             this._listener._filterFunction = filterFunction;
           } else {
             folders = aFolders;
+            this._listener._filterFunction = function(folder) { return true; };
           }
 
           if (excludeServers.length > 0) {
             folders = folders.filter(function(aFolder) {
               return !(excludeServers.indexOf(aFolder.server.key) != -1); });
           }
 
           /* This code block will do the following: Add a menu item that refers
@@ -425,23 +435,23 @@
             }
           }
 
           let globalInboxFolder = null;
           // See if this is the toplevel menu (usually with accounts).
           if (!this._parentFolder) {
             let addSeparator = false;
             // Some menus want a "Recent" option, but that should only be on our
-            // top-level menu
+            // top-level menu.
             if (this.getAttribute("showRecent") == "true") {
-              this._buildRecentMenu();
+              this._buildSpecialMenu("recent");
               addSeparator = true;
             }
             if (this.getAttribute("showFavorites") == "true") {
-              this._buildFavoritesMenu();
+              this._buildSpecialMenu("favorites");
               addSeparator = true;
             }
             if (addSeparator) {
               // If we added Recent and/or Favorites, separate them from the rest of the items.
               let sep = document.createElement("menuseparator");
               sep.setAttribute("generated", "true");
               this.appendChild(sep);
             }
@@ -582,165 +592,125 @@
             }
             node.setAttribute("label", label);
             this._setCssSelectors(folder, node);
           }
         ]]></body>
       </method>
 
       <!--
+         - This only builds the menu item in the top-level menulist.
+         - The real submenu will be created once the popup is really shown,
+         - via the _buildSpecialSubmenu method.
+         -->
+      <method name="_buildSpecialMenu">
+        <parameter name="type"/>
+        <body><![CDATA[
+          // Now create the Recent folder and its children
+          let menu = document.createElement("menu");
+          if (type == "recent") {
+            menu.setAttribute("label", this.getAttribute("recentLabel"));
+            menu.setAttribute("accesskey", this.getAttribute("recentAccessKey"));
+          } else {
+            menu.setAttribute("label", this.getAttribute("favoritesLabel"));
+            menu.setAttribute("accesskey", this.getAttribute("favoritesAccessKey"));
+          }
+          menu.setAttribute("special", type);
+          menu.setAttribute("generated", "true");
+
+          let popup = document.createElement("menupopup");
+          popup.setAttribute("class", this.getAttribute("class"));
+          popup.addEventListener("popupshowing",
+                                 () => this._buildSpecialSubmenu(menu),
+                                 { once: true });
+
+          menu.appendChild(popup);
+          this.appendChild(menu);
+        ]]></body>
+      </method>
+
+      <!--
          - Builds a submenu with all of the recently used folders in it, to
          - allow for easy access.
         -->
-      <method name="_buildRecentMenu">
+      <method name="_buildSpecialSubmenu">
+        <parameter name="menu"/>
         <body><![CDATA[
-          // Iterate through all folders in all accounts, and find 15 (_MAXRECENT)
-          // of most recently modified ones.
-          let allFolders = this.toArray(
+          let specialType = menu.getAttribute("special");
+          if (this._initializedSpecials.has(specialType))
+            return;
+
+          // Iterate through all folders in all accounts matching the current filter.
+          let specialFolders = this.toArray(
             this.fixIterator(this.MailServices.accounts.allFolders, Ci.nsIMsgFolder));
-
-          allFolders = allFolders.filter(f => f.canFileMessages);
+          if (this._listener._filterFunction)
+            specialFolders = specialFolders.filter(this._listener._filterFunction);
 
-          let recentFolders = this.getMostRecentFolders(allFolders,
-                                                        this._MAXRECENT,
-                                                        "MRMTime");
+          switch (specialType) {
+            case "recent":
+              // Find 15 (_MAXRECENT) of most recently modified ones.
+              specialFolders = this.getMostRecentFolders(specialFolders,
+                                                         this._MAXRECENT,
+                                                         "MRMTime");
+              break;
+            case "favorites":
+              specialFolders = specialFolders.filter(folder => folder.getFlag(Ci.nsMsgFolderFlags.Favorite));
+              break;
+          }
 
           // Cache the pretty names so that they do not need to be fetched
           // _MAXRECENT^2 times later.
-          recentFolders = recentFolders.map(
+          let specialFoldersMap = specialFolders.map(
             function (f) { return { folder: f, name: f.prettyName } });
 
           // Because we're scanning across multiple accounts, we can end up with
           // several folders with the same name. Find those dupes.
           let dupeNames = new Set();
-          for (let i = 0; i < recentFolders.length; i++) {
-            for (let j = i + 1; j < recentFolders.length; j++) {
-              if (recentFolders[i].name == recentFolders[j].name)
-                dupeNames.add(recentFolders[i].name);
+          for (let i = 0; i < specialFoldersMap.length; i++) {
+            for (let j = i + 1; j < specialFoldersMap.length; j++) {
+              if (specialFoldersMap[i].name == specialFoldersMap[j].name)
+                dupeNames.add(specialFoldersMap[i].name);
             }
           }
 
-          for (let folderItem of recentFolders) {
+          for (let folderItem of specialFoldersMap) {
             // If this folder name appears multiple times in the recent list,
             // append the server name to disambiguate.
             // TODO:
             // - maybe this could use verboseFolderFormat from messenger.properties
             //   instead of hardcoded " - ".
             // - disambiguate folders with same name in same account
             //   (in different subtrees).
             let label = folderItem.name;
             if (dupeNames.has(label))
               label += " - " + folderItem.folder.server.prettyName;
 
             folderItem.label = label;
           }
 
           // Make sure the entries are sorted alphabetically.
-          recentFolders.sort((a, b) => this.folderNameCompare(a.label, b.label));
-
-          // Now create the Recent folder and its children
-          var menu = document.createElement("menu");
-          menu.setAttribute("label", this.getAttribute("recentLabel"));
-          menu.setAttribute("accesskey", this.getAttribute("recentAccessKey"));
-          var popup = document.createElement("menupopup");
-          popup.setAttribute("class", this.getAttribute("class"));
-          popup.setAttribute("generated", "true");
-          menu.appendChild(popup);
+          specialFoldersMap.sort((a, b) => this.folderNameCompare(a.label, b.label));
 
           // Create entries for each of the recent folders.
-          for (let folderItem of recentFolders) {
+          for (let folderItem of specialFoldersMap) {
             let node = document.createElement("menuitem");
 
             node.setAttribute("label", folderItem.label);
             node._folder = folderItem.folder;
 
             node.setAttribute("class", "folderMenuItem menuitem-iconic");
             this._setCssSelectors(folderItem.folder, node);
             node.setAttribute("generated", "true");
-            popup.appendChild(node);
-          }
-          menu.setAttribute("generated", "true");
-          this.appendChild(menu);
-          if (!recentFolders.length)
-            menu.setAttribute("disabled", "true");
-        ]]></body>
-      </method>
-
-      <!--
-         - Builds a submenu with all of the favorite folders in it, to
-         - allow for easy access.
-        -->
-      <method name="_buildFavoritesMenu">
-        <body><![CDATA[
-          // Iterate through all folders in all accounts, and find the
-          // favorite ones.
-          let allFolders = this.toArray(
-            this.fixIterator(this.MailServices.accounts.allFolders, Ci.nsIMsgFolder));
-
-          let favoriteFolders = allFolders.filter(f => f.canFileMessages &&
-                                                       f.getFlag(Ci.nsMsgFolderFlags.Favorite));
-
-          // Cache the pretty names so that they do not need to be fetched
-          // _MAXFAVORITES^2 times later.
-          favoriteFolders = favoriteFolders.map(
-            function (f) { return { folder: f, name: f.prettyName } });
-
-          // Because we're scanning across multiple accounts, we can end up with
-          // several folders with the same name. Find those dupes.
-          let dupeNames = new Set();
-          for (let i = 0; i < favoriteFolders.length; i++) {
-            for (let j = i + 1; j < favoriteFolders.length; j++) {
-              if (favoriteFolders[i].name == favoriteFolders[j].name)
-                dupeNames.add(favoriteFolders[i].name);
-            }
+            menu.menupopup.appendChild(node);
           }
 
-          for (let folderItem of favoriteFolders) {
-            // If this folder name appears multiple times in the favorites list,
-            // append the server name to disambiguate.
-            // TODO:
-            // - maybe this could use verboseFolderFormat from messenger.properties
-            //   instead of hardcoded " - ".
-            // - disambiguate folders with same name in same account
-            //   (in different subtrees).
-            let label = folderItem.name;
-            if (dupeNames.has(label))
-              label += " - " + folderItem.folder.server.prettyName;
-
-            folderItem.label = label;
-          }
-
-          // Make sure the entries are sorted alphabetically.
-          favoriteFolders.sort((a, b) => this.folderNameCompare(a.label, b.label));
+          if (specialFoldersMap.length == 0)
+            menu.setAttribute("disabled", "true");
 
-          // Now create the Favorites folder and its children
-          var menu = document.createElement("menu");
-          menu.setAttribute("label", this.getAttribute("favoritesLabel"));
-          menu.setAttribute("accesskey", this.getAttribute("favoritesAccessKey"));
-          var popup = document.createElement("menupopup");
-          popup.setAttribute("class", this.getAttribute("class"));
-          popup.setAttribute("generated", "true");
-          menu.appendChild(popup);
-
-          // Create entries for each of the favorite folders.
-          for (let folderItem of favoriteFolders) {
-            let node = document.createElement("menuitem");
-
-            node.setAttribute("label", folderItem.label);
-            node._folder = folderItem.folder;
-
-            node.setAttribute("class", "folderMenuItem menuitem-iconic");
-            this._setCssSelectors(folderItem.folder, node);
-            node.setAttribute("generated", "true");
-            popup.appendChild(node);
-          }
-          menu.setAttribute("generated", "true");
-          this.appendChild(menu);
-          if (!favoriteFolders.length)
-            menu.setAttribute("disabled", "true");
+          this._initializedSpecials.add(specialType);
         ]]></body>
       </method>
 
       <!--
          - This function adds attributes on menu/menuitems to make it easier for
          - css to style them.
          -
          - @param aFolder    the folder that corresponds to the menu/menuitem
@@ -877,28 +847,32 @@
 
       <!--
          - Removes all menu-items for this popup, resets all fields, and
          - removes the listener.  This function is invoked when a change
          - that affects this menu is detected by our listener.
         -->
       <method name="_teardown">
         <body><![CDATA[
+          if (!this._initialized)
+            return;
+
           for (let i = this.childNodes.length - 1; i >= 0; i--) {
             let child = this.childNodes[i];
             if (child.getAttribute("generated") != "true")
               continue;
             if ("_teardown" in child)
               child._teardown();
             child.remove();
           }
 
           this._removeListener();
 
           this._initialized = false;
+          this._initializedSpecials.clear();
         ]]></body>
       </method>
     </implementation>
 
     <handlers>
       <!--
          - In order to improve performance, we're not going to build any of the
          - menu until we're shown (unless we're the child of a menulist, see