Bug 533775 - Keep Favorite an Unread folders view in the same order as All folders view, show account name nodes and do not persist open state for them + test. ui-r=bwinton, r=mkmelin
authoraceman <acelists@atlas.sk>
Tue, 25 Feb 2014 08:22:23 -0500
changeset 17608 b654c1648f7d26bb63be4397707769a6fec9ae10
parent 17607 6e585f3aa13801cee18a99982c1a78506e04ee59
child 17609 c370a7d5679b0b34bf2fae72f5381b95ff8e2eeb
push id1175
push usermbanner@mozilla.com
push dateTue, 18 Mar 2014 08:37:15 +0000
treeherdercomm-aurora@3b5242ee031a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwinton, mkmelin
bugs533775
Bug 533775 - Keep Favorite an Unread folders view in the same order as All folders view, show account name nodes and do not persist open state for them + test. ui-r=bwinton, r=mkmelin
mail/base/content/folderPane.js
mail/base/content/mailWidgets.xml
mail/test/mozmill/folder-display/test-display-message-with-folder-modes.js
mail/test/mozmill/folder-display/test-folder-names-in-favorite-mode.js
mail/test/mozmill/folder-pane/test-display-message-with-folder-modes.js
mail/test/mozmill/folder-pane/test-folder-names-in-recent-mode.js
mail/test/mozmill/folder-pane/test-folder-pane.js
mail/test/mozmill/shared-modules/test-folder-display-helpers.js
--- a/mail/base/content/folderPane.js
+++ b/mail/base/content/folderPane.js
@@ -717,23 +717,30 @@ let gFolderTreeView = {
    */
   getCellText: function ftv_getCellText(aRow, aCol) {
     if (aCol.id == "folderNameCol")
       return this._rowMap[aRow].text;
     return "";
   },
 
   /**
-   * The ftvItems take care of assigning this when building children lists
+   * The ftvItems take care of assigning this when created.
    */
   getLevel: function ftv_getLevel(aIndex) {
     return this._rowMap[aIndex].level;
   },
 
   /**
+   * The ftvItems take care of assigning this when building children lists
+   */
+  getServerNameAdded: function ftv_getServerNameAdded(aIndex) {
+    return this._rowMap[aIndex].addServerName;
+  },
+
+  /**
    * This is easy since the ftv items assigned the _parent property when making
    * the child lists
    */
   getParentIndex: function ftv_getParentIndex(aIndex) {
     return this._rowMap.indexOf(this._rowMap[aIndex]._parent);
   },
 
   /**
@@ -836,40 +843,33 @@ let gFolderTreeView = {
       let row = this._rowMap[i];
       while (row && row.level > this._rowMap[aIndex].level) {
         count++;
         row = this._rowMap[++i];
       }
       this._rowMap.splice(aIndex + 1, count);
 
       // Remove us from the persist map
-      let index = this._persistOpenMap[this.mode]
-                      .indexOf(this._rowMap[aIndex].id);
-      if (index != -1)
-        this._persistOpenMap[this.mode].splice(index, 1);
+      this._persistItemClosed(this._rowMap[aIndex].id);
 
       // Notify the tree of changes
       if (this._tree) {
         this._tree.rowCountChanged(aIndex + 1, (-1) * count);
         this._tree.invalidateRow(aIndex);
       }
     } else {
       // We're opening the container.  Add the children to our map
 
       // Note that these children may have been open when we were last closed,
       // and if they are, we also have to add those grandchildren to the map
       let oldCount = this._rowMap.length;
       this.recursivelyAddToMap(this._rowMap[aIndex], aIndex);
 
       // Add this folder to the persist map
-      if (!this._persistOpenMap[this.mode])
-        this._persistOpenMap[this.mode] = [];
-      let id = this._rowMap[aIndex].id;
-      if (this._persistOpenMap[this.mode].indexOf(id) == -1)
-        this._persistOpenMap[this.mode].push(id);
+      this._persistItemOpen(this._rowMap[aIndex].id);
 
       // Notify the tree of changes
       if (this._tree) {
         this._tree.rowCountChanged(aIndex + 1, this._rowMap.length - oldCount);
         this._tree.invalidateRow(aIndex);
       }
       // if this was a server that was expanded, let it update its counts
       let folder = this._rowMap[aIndex]._folder;
@@ -1083,52 +1083,95 @@ let gFolderTreeView = {
   /**
    * This is an array of all possible modes for the folder tree. You should not
    * modify this directly, but rather use registerFolderTreeMode.
    */
   _modeNames: ["all", "unread", "favorite", "recent", "smart"],
   _modeDisplayNames: {},
 
   /**
-   * This is a javaascript map of which folders we had open, so that we can
+   * This is a javascript map of which folders we had open, so that we can
    * persist their state over-time.  It is designed to be used as a JSON object.
    */
   _persistOpenMap: {},
+  _notPersistedModes: ["unread", "favorite", "recent"],
 
+  /**
+   * Iterate over the persistent list and open the items (folders) stored in it.
+   */
   _restoreOpenStates: function ftv__persistOpenStates() {
-    if (!(this.mode in this._persistOpenMap))
-      return;
+    let mode = this.mode;
+    // Remove any saved state of modes where open state should not be persisted.
+    // This is mostly for migration from older profiles that may have the info stored.
+    if (this._notPersistedModes.indexOf(mode) != -1) {
+      delete this._persistOpenMap[mode];
+    }
 
     let curLevel = 0;
     let tree = this;
+    let map = tree._persistOpenMap[mode]; // may be undefined
     function openLevel() {
       let goOn = false;
       // We can't use a js iterator because we're changing the array as we go.
       // So fallback on old trick of going backwards from the end, which
       // doesn't care when you add things at the end.
       for (let i = tree._rowMap.length - 1; i >= 0; i--) {
         let row = tree._rowMap[i];
         if (row.level != curLevel)
           continue;
 
-        let map = tree._persistOpenMap[tree.mode];
-        if (map && map.indexOf(row.id) != -1) {
+        // The initial state of all rows is closed, so toggle those we want open.
+        if (!map || map.indexOf(row.id) != -1) {
           tree._toggleRow(i, false);
           goOn = true;
         }
       }
 
       // If we opened up any new kids, we need to check their level as well.
       curLevel++;
       if (goOn)
         openLevel();
     }
     openLevel();
   },
 
+  /**
+   * Remove the item from the persistent list, meaning the item should
+   * be persisted as closed in the tree.
+   *
+   * @param aItemId  The URI of the folder item.
+   */
+  _persistItemClosed: function ftv_unpersistItem(aItemId) {
+    let mode = this.mode;
+    if (this._notPersistedModes.indexOf(mode) != -1)
+      return;
+
+    let persistMapIndex = this._persistOpenMap[mode].indexOf(aItemId);
+    if (persistMapIndex != -1)
+      this._persistOpenMap[mode].splice(persistMapIndex, 1);
+  },
+
+  /**
+   * Add the item from the persistent list, meaning the item should
+   * be persisted as open (expanded) in the tree.
+   *
+   * @param aItemId  The URI of the folder item.
+   */
+  _persistItemOpen: function ftv_persistItem(aItemId) {
+    let mode = this.mode;
+    if (this._notPersistedModes.indexOf(mode) != -1)
+      return;
+
+    if (!this._persistOpenMap[mode])
+      this._persistOpenMap[mode] = [];
+
+    if (this._persistOpenMap[mode].indexOf(aItemId) == -1)
+      this._persistOpenMap[mode].push(aItemId);
+  },
+
   _tree: null,
   selection: null,
   /**
    * An array of ftvItems, where each item corresponds to a row in the tree
    */
   _rowMap: null,
 
   /**
@@ -1212,79 +1255,66 @@ let gFolderTreeView = {
      * The unread mode returns all folders that are not root-folders and that
      * have unread items.  Also always keep the currently selected folder
      * so it doesn't disappear under the user.
      */
     unread: {
       __proto__: IFolderTreeMode,
 
       generateMap: function ftv_unread_generateMap(ftv) {
-        let map = [];
-        let currentFolder = gFolderTreeView.getSelectedFolders()[0];
-        const outFolderFlagMask = nsMsgFolderFlags.SentMail |
-          nsMsgFolderFlags.Drafts | nsMsgFolderFlags.Queue |
-          nsMsgFolderFlags.Templates;
-        for each (let folder in ftv._enumerateFolders) {
-          if (!folder.isSpecialFolder(outFolderFlagMask, true) &&
-              (!folder.isServer && folder.getNumUnread(false) > 0) ||
-              (folder == currentFolder))
-            map.push(new ftvItem(folder));
+        let filterUnread = function filterUnread(aFolder) {
+          let currentFolder = gFolderTreeView.getSelectedFolders()[0];
+          const outFolderFlagMask = nsMsgFolderFlags.SentMail |
+            nsMsgFolderFlags.Drafts | nsMsgFolderFlags.Queue |
+            nsMsgFolderFlags.Templates;
+          return (!aFolder.isSpecialFolder(outFolderFlagMask, true) &&
+                  ((aFolder.getNumUnread(true) > 0) ||
+                   (aFolder == currentFolder)))
         }
 
-        // There are no children in this view!
-        for each (let folder in map) {
-          folder.__defineGetter__("children", function() []);
-          folder.addServerName = true;
+        let accounts = gFolderTreeView._sortedAccounts();
+        // Force each root folder to do its local subfolder discovery.
+        MailUtils.discoverFolders();
+
+        let unreadRootFolders = [];
+        for (let acct of accounts) {
+          let rootFolder = acct.incomingServer.rootFolder;
+          // Add rootFolders of accounts that contain at least one Favorite folder.
+          if (rootFolder.getNumUnread(true) > 0)
+            unreadRootFolders.push(new ftvItem(rootFolder, filterUnread));
         }
-        sortFolderItems(map);
-        return map;
-      },
 
-      getParentOfFolder: function ftv_unread_getParentOfFolder(aFolder) {
-        // This is a flat view, so no folders have parents.
-        return null;
+        return unreadRootFolders;
       }
     },
 
     /**
      * The favorites mode returns all folders whose flags are set to include
-     * the favorite flag
+     * the favorite flag.
      */
     favorite: {
       __proto__: IFolderTreeMode,
 
       generateMap: function ftv_favorite_generateMap(ftv) {
-        let faves = [];
-        for each (let folder in ftv._enumerateFolders) {
-          if (folder.flags & nsMsgFolderFlags.Favorite)
-            faves.push(new ftvItem(folder));
+        let accounts = gFolderTreeView._sortedAccounts();
+        // Force each root folder to do its local subfolder discovery.
+        MailUtils.discoverFolders();
+
+        let favRootFolders = [];
+        let filterFavorite = function filterFavorite(aFolder) {
+          return aFolder.getFolderWithFlags(nsMsgFolderFlags.Favorite) != null;
+        }
+        for (let acct of accounts) {
+          let rootFolder = acct.incomingServer.rootFolder;
+          // Add rootFolders of accounts that contain at least one Favorite folder.
+          if (filterFavorite(rootFolder))
+            favRootFolders.push(new ftvItem(rootFolder, filterFavorite));
         }
 
-        // There are no children in this view!
-        // And we want to display the account name to distinguish folders w/
-        // the same name. (only for folders with duplicated names)
-        let uniqueNames = new Object();
-        for each (let item in faves) {
-          let name = item._folder.abbreviatedName.toLowerCase();
-          item.__defineGetter__("children", function() []);
-          if (!uniqueNames[name])
-            uniqueNames[name] = 0;
-          uniqueNames[name]++;
-        }
-        for each (let item in faves) {
-          let name = item._folder.abbreviatedName.toLowerCase();
-          item.addServerName = (uniqueNames[name] > 1);
-        }
-        sortFolderItems(faves);
-        return faves;
-      },
-
-      getParentOfFolder: function ftv_unread_getParentOfFolder(aFolder) {
-        // This is a flat view, so no folders have parents.
-        return null;
+        return favRootFolders;
       }
     },
 
     recent: {
       __proto__: IFolderTreeMode,
 
       generateMap: function ftv_recent_generateMap(ftv) {
         const MAXRECENT = 15;
@@ -1703,16 +1733,21 @@ let gFolderTreeView = {
         this._rebuild();
       return;
     }
     this._modes[this._mode].onFolderAdded(
       aParentItem.QueryInterface(Components.interfaces.nsIMsgFolder), aItem);
   },
   addFolder: function ftl_add_folder(aParentItem, aItem)
   {
+    // This intentionally adds any new folder even if it would not pass the
+    // _filterFunction. The idea is that the user can add new folders even
+    // in modes like "unread" or "favorite" and could wonder why they
+    // are not appearing (forgetting they do not meet the criteria of the view).
+    // The folders will be hidden properly next time the view is rebuilt.
     let parentIndex = this.getIndexOfFolder(aParentItem);
     let parent = this._rowMap[parentIndex];
     if (!parent)
        return;
 
     // Getting these children might have triggered our parent to build its
     // array just now, in which case the added item will already exist
     let children = parent.children;
@@ -1746,19 +1781,17 @@ let gFolderTreeView = {
     }
     this._addChildToView(parent, parentIndex, newChild);
   },
 
   OnItemRemoved: function ftl_remove(aRDFParentItem, aItem) {
     if (!(aItem instanceof Components.interfaces.nsIMsgFolder))
       return;
 
-    let persistMapIndex = this._persistOpenMap[this.mode].indexOf(aItem.URI);
-    if (persistMapIndex != -1)
-      this._persistOpenMap[this.mode].splice(persistMapIndex, 1);
+    this._persistItemClosed(aItem.URI);
 
     let index = this.getIndexOfFolder(aItem);
     if (index == null)
       return;
     // forget our parent's children; they'll get rebuilt
     if (aRDFParentItem)
       this._rowMap[index]._parent._children = null;
     let kidCount = 1;
@@ -1770,21 +1803,26 @@ let gFolderTreeView = {
     }
     this._rowMap.splice(index, kidCount);
     this._tree.rowCountChanged(index, -1 * kidCount);
     this._tree.invalidateRow(index);
   },
 
   OnItemPropertyChanged: function(aItem, aProperty, aOld, aNew) {},
   OnItemIntPropertyChanged: function(aItem, aProperty, aOld, aNew) {
-    // we want to rebuild only if we're in unread mode, and we have a
+    // We want to rebuild only if we're in unread mode, and we have a
     // newly unread folder, and we didn't already have the folder.
-    if (this._mode == "unread" &&
-        aProperty == "TotalUnreadMessages" && aOld == 0 &&
-        this.getIndexOfFolder(aItem) == null) {
+    // Or if we are in favorite view and the favorite status of a folder changed.
+    if ((this._mode == "unread" &&
+         aProperty == "TotalUnreadMessages" && aOld == 0 &&
+         this.getIndexOfFolder(aItem) == null) ||
+        (this._mode == "favorite" && aProperty == "FolderFlag" &&
+         (aOld & Components.interfaces.nsMsgFolderFlags.Favorite) !=
+         (aNew & Components.interfaces.nsMsgFolderFlags.Favorite)))
+    {
       this._rebuild();
       return;
     }
 
     if (aItem instanceof Components.interfaces.nsIMsgFolder) {
       let index = this.getIndexOfFolder(aItem);
       let folder = aItem;
       let folderTreeMode = this._modes[this._mode];
@@ -1825,19 +1863,30 @@ let gFolderTreeView = {
  * level (attribute) - the level in the tree to display the item at
  * open (rw, attribute) - whether or not this container is open
  * children (attribute) - an array of child items also conforming to this spec
  * getProperties (function) - a call from getRowProperties or getCellProperties
  *                            for this item will be passed into this function
  * command (function) - this function will be called when the item is double-
  *                      clicked
  */
-function ftvItem(aFolder) {
+
+/**
+ * The ftvItem constructor takes these arguments:
+ *
+ * @param aFolder        The folder attached to this row in the tree.
+ * @param aFolderFilter  When showing children folders of this one,
+ *                       only show those that pass this filter function.
+ *                       If unset, show all subfolders.
+ */
+function ftvItem(aFolder, aFolderFilter) {
   this._folder = aFolder;
   this._level = 0;
+  this._parent = null;
+  this._folderFilter = aFolderFilter;
 }
 
 ftvItem.prototype = {
   open: false,
   addServerName: false,
   useServerNameOnly: false,
 
   get id() {
@@ -1898,21 +1947,27 @@ ftvItem.prototype = {
       let iter;
       try {
         iter = fixIterator(this._folder.subFolders, Ci.nsIMsgFolder);
       } catch (ex) {
         Services.console.logStringMessage("Discovering children for " + this._folder.URI +
                                           " failed with " + "exception: " + ex);
         iter = [];
       }
-      this._children = [new ftvItem(f) for each (f in iter)];
-
+      this._children = [];
+      // Out of all children, only keep those that match the _folderFilter
+      // and those that contain such children.
+      for (let folder in iter) {
+        if (!this._folderFilter || this._folderFilter(folder)) {
+          this._children.push(new ftvItem(folder, this._folderFilter));
+        }
+      }
       sortFolderItems(this._children);
       // Each child is a level one below us
-      for each (let child in this._children) {
+      for (let child of this._children) {
         child._level = this._level + 1;
         child._parent = this;
       }
     }
     return this._children;
   }
 };
 
--- a/mail/base/content/mailWidgets.xml
+++ b/mail/base/content/mailWidgets.xml
@@ -2541,18 +2541,23 @@
         </body>
       </method>
 
       <method name="addLocationInfo">
         <parameter name="aFolder"/>
         <parameter name="aCropped"/>
         <body>
           <![CDATA[
-            // Display location info for items in favorite folder pane mode.
-            if (gFolderTreeView.mode == "favorite") {
+            // Display also server name for items that are on level 0 and are not server names
+            // by themselves and do not have server name already appended in their label.
+            let folderIndex = gFolderTreeView.getIndexOfFolder(aFolder);
+            if (!aFolder.isServer &&
+                gFolderTreeView.getLevel(folderIndex) == 0 &&
+                !gFolderTreeView.getServerNameAdded(folderIndex))
+            {
               let loc = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "folderSummaryLocation");
               let midPath = "";
               let midFolder = aFolder.parent;
               while (aFolder.server.rootFolder != midFolder) {
                 midPath = midFolder.name + " - " + midPath;
                 midFolder = midFolder.parent;
               }
               loc.setAttribute("location", aFolder.server.prettyName + " - " + midPath + aFolder.name);
deleted file mode 100644
--- a/mail/test/mozmill/folder-display/test-display-message-with-folder-modes.js
+++ /dev/null
@@ -1,183 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-/*
- * Test that displaying messages in folder tabs works correctly with folder
- * modes. This includes:
- * - switching to the default folder mode if the folder isn't present in the
- *   current folder mode
- * - not switching otherwise
- * - making sure that we're able to expand the right folders in the smart folder
- *   mode
- */
-
-var MODULE_NAME = "test-display-message-with-folder-modes";
-
-var RELATIVE_ROOT = "../shared-modules";
-var MODULE_REQUIRES = ["folder-display-helpers"];
-
-var folder;
-var dummyFolder;
-var smartInboxFolder;
-
-var msgHdr;
-
-function setupModule(module) {
-  let fdh = collector.getModule("folder-display-helpers");
-  fdh.installInto(module);
-
-  // This is a subfolder of the inbox so that
-  // test_display_message_in_smart_folder_mode_works is able to test that we
-  // don't attempt to expand any inboxes.
-  inboxFolder.createSubfolder("DisplayMessageWithFolderModesA", null);
-  folder = inboxFolder.getChildNamed("DisplayMessageWithFolderModesA");
-  // This second folder is meant to act as a dummy folder to switch to when we
-  // want to not be in folder.
-  inboxFolder.createSubfolder("DisplayMessageWithFolderModesB", null);
-  dummyFolder = inboxFolder.getChildNamed("DisplayMessageWithFolderModesB");
-  make_new_sets_in_folder(folder, [{count: 5}]);
-  // The message itself doesn't really matter, as long as there's at least one
-  // in the inbox.  We will delete this in teardownModule because the inbox
-  // is a shared resource and it's not okay to leave stuff in there.
-  make_new_sets_in_folder(inboxFolder, [{count: 1}]);
-}
-
-/**
- * Test that displaying a message causes a switch to the default folder mode if
- * the folder isn't present in the current folder mode.
- */
-function test_display_message_with_folder_not_present_in_current_folder_mode() {
-  be_in_folder(folder);
-  msgHdr = mc.dbView.getMsgHdrAt(0);
-
-  // Make sure the folder doesn't appear in the favorite folder mode just
-  // because it was selected last before switching
-  be_in_folder(inboxFolder);
-
-  // Move to favorite folders. This folder isn't currently a favorite folder
-  mc.folderTreeView.mode = "favorite";
-  assert_folder_not_visible(folder);
-
-  // Try displaying a message
-  display_message_in_folder_tab(msgHdr);
-
-  assert_folder_mode(mc.window.kDefaultMode);
-  assert_folder_selected_and_displayed(folder);
-  assert_selected_and_displayed(msgHdr);
-}
-
-/**
- * Test that displaying a message _does not_ cause a switch to the default
- * folder mode if the folder is present in the current folder mode.
- */
-function test_display_message_with_folder_present_in_current_folder_mode() {
-  // Mark the folder as a favorite
-  folder.flags |= Ci.nsMsgFolderFlags.Favorite;
-  // Also mark the dummy folder as a favorite, in preparation for
-  // test_display_message_in_smart_folder_mode_works
-  dummyFolder.flags |= Ci.nsMsgFolderFlags.Favorite;
-
-  // Make sure the folder doesn't appear in the favorite folder mode just
-  // because it was selected last before switching
-  be_in_folder(inboxFolder);
-
-  // Switch to favorite folders. Check that the folder is now in the view, as is
-  // the dummy folder
-  mc.folderTreeView.mode = "favorite";
-  assert_folder_visible(folder);
-  assert_folder_visible(dummyFolder);
-
-  // Try displaying a message
-  display_message_in_folder_tab(msgHdr);
-
-  assert_folder_mode("favorite");
-  assert_folder_selected_and_displayed(folder);
-  assert_selected_and_displayed(msgHdr);
-
-  // Now unset the flags so that we don't affect later tests.
-  folder.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-  dummyFolder.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-}
-
-/**
- * Test that displaying a message in smart folders mode causes the parent in the
- * view to expand.
- */
-function test_display_message_in_smart_folder_mode_works() {
-  // Clear the message selection, otherwise msgHdr will still be displayed and
-  // display_message_in_folder_tab(msgHdr) will be a no-op.
-  select_none();
-  // Switch to the dummy folder, otherwise msgHdr will be in the view and the
-  // display message in folder tab logic will simply select the message without
-  // bothering to expand any folders.
-  be_in_folder(dummyFolder);
-
-  mc.folderTreeView.mode = "smart";
-
-  let rootFolder = folder.server.rootFolder;
-  // Check that the folder is actually the child of the account root
-  assert_folder_child_in_view(folder, rootFolder);
-
-  // Collapse everything
-  smartInboxFolder = get_smart_folder_named("Inbox");
-  collapse_folder(smartInboxFolder);
-  assert_folder_collapsed(smartInboxFolder);
-  collapse_folder(rootFolder);
-  assert_folder_collapsed(rootFolder);
-  assert_folder_not_visible(folder);
-
-  // Try displaying the message
-  display_message_in_folder_tab(msgHdr);
-
-  // Check that the right folders have expanded
-  assert_folder_mode("smart");
-  assert_folder_collapsed(smartInboxFolder);
-  assert_folder_expanded(rootFolder);
-  assert_folder_selected_and_displayed(folder);
-  assert_selected_and_displayed(msgHdr);
-}
-
-/**
- * Test that displaying a message in an inbox in smart folders mode causes the
- * message to be displayed in the smart inbox.
- */
-function test_display_inbox_message_in_smart_folder_mode_works() {
-  be_in_folder(inboxFolder);
-  let inboxMsgHdr = mc.dbView.getMsgHdrAt(0);
-
-  // Collapse everything
-  collapse_folder(smartInboxFolder);
-  assert_folder_collapsed(smartInboxFolder);
-  assert_folder_not_visible(inboxFolder);
-  let rootFolder = folder.server.rootFolder;
-  collapse_folder(rootFolder);
-  assert_folder_collapsed(rootFolder);
-
-  // Move to a different folder
-  be_in_folder(get_smart_folder_named("Trash"));
-  assert_message_not_in_view(inboxMsgHdr);
-
-  // Try displaying the message
-  display_message_in_folder_tab(inboxMsgHdr);
-
-  // Check that nothing has expanded, and that the right folder is selected
-  assert_folder_mode("smart");
-  assert_folder_collapsed(smartInboxFolder);
-  assert_folder_collapsed(rootFolder);
-  assert_folder_selected_and_displayed(smartInboxFolder);
-  assert_selected_and_displayed(inboxMsgHdr);
-}
-
-/**
- * Move back to the all folders mode.
- */
-function test_switch_to_all_folders() {
-  mc.folderTreeView.mode = "all";
-}
-
-function teardownModule() {
-  be_in_folder(inboxFolder);
-  select_click_row(0);
-  press_delete();
-}
deleted file mode 100644
--- a/mail/test/mozmill/folder-display/test-folder-names-in-favorite-mode.js
+++ /dev/null
@@ -1,46 +0,0 @@
-var MODULE_NAME = "test-folder-names-in-favorite-mode";
-var RELATIVE_ROOT = "../shared-modules";
-var MODULE_REQUIRES = ["folder-display-helpers"];
-
-function setupModule(module) {
-  let fdh = collector.getModule("folder-display-helpers");
-  fdh.installInto(module);
-};
-
-function test_folder_names_in_favorite_pane_view_mode() {
-  let acc1 = MailServices.accounts.accounts.queryElementAt(0, Ci.nsIMsgAccount);
-  let acc2 = MailServices.accounts.accounts.queryElementAt(1, Ci.nsIMsgAccount);
-  let rootFolder1 = acc1.incomingServer.rootFolder;
-  let rootFolder2 = acc2.incomingServer.rootFolder;
-
-  rootFolder1.createSubfolder("uniqueName", null);
-  rootFolder1.createSubfolder("duplicatedName", null);
-  rootFolder2.createSubfolder("duplicatedName", null);
-  rootFolder2.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox).createSubfolder("duplicatedName", null);
-
-  let fUnique = rootFolder1.getChildNamed("uniqueName");
-  let fDup1 = rootFolder1.getChildNamed("duplicatedName");
-  let fDup2 = rootFolder2.getChildNamed("duplicatedName");
-  let fDup3 = rootFolder2.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox).getChildNamed("duplicatedName");
-  let inbox = rootFolder2.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox);
-
-  fUnique.flags |= Ci.nsMsgFolderFlags.Favorite;
-  fDup1.flags |= Ci.nsMsgFolderFlags.Favorite;
-  fDup2.flags |= Ci.nsMsgFolderFlags.Favorite;
-  fDup3.flags |= Ci.nsMsgFolderFlags.Favorite;
-  inbox.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-
-  mc.window.gFolderTreeView.mode = "favorite";
-
-  assert_folder_at_index_as(0, "duplicatedName - Local Folders");
-  assert_folder_at_index_as(1, "duplicatedName - tinderbox@foo.invalid");
-  assert_folder_at_index_as(2, "duplicatedName - tinderbox@foo.invalid");
-  assert_folder_at_index_as(3, "uniqueName");
-
-  // Reset the flags to the state they were previously.
-  fUnique.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-  fDup1.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-  fDup2.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-  fDup3.flags &= ~Ci.nsMsgFolderFlags.Favorite;
-  inbox.flags |= Ci.nsMsgFolderFlags.Favorite;
-};
new file mode 100644
--- /dev/null
+++ b/mail/test/mozmill/folder-pane/test-display-message-with-folder-modes.js
@@ -0,0 +1,202 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/*
+ * Test that displaying messages in folder tabs works correctly with folder
+ * modes. This includes:
+ * - switching to the default folder mode if the folder isn't present in the
+ *   current folder mode
+ * - not switching otherwise
+ * - making sure that we're able to expand the right folders in the smart folder
+ *   mode
+ */
+
+const MODULE_NAME = "test-display-message-with-folder-modes";
+
+const RELATIVE_ROOT = "../shared-modules";
+const MODULE_REQUIRES = ["folder-display-helpers"];
+
+var folder;
+var dummyFolder;
+var inbox2Folder;
+var smartInboxFolder;
+
+var msgHdr;
+
+function setupModule(module) {
+  collector.getModule("folder-display-helpers").installInto(module);
+
+  assert_folder_mode("all");
+  assert_folder_tree_view_row_count(7);
+
+  // This is a subfolder of the inbox so that
+  // test_display_message_in_smart_folder_mode_works is able to test that we
+  // don't attempt to expand any inboxes.
+  inboxFolder.createSubfolder("DisplayMessageWithFolderModesA", null);
+  folder = inboxFolder.getChildNamed("DisplayMessageWithFolderModesA");
+  // This second folder is meant to act as a dummy folder to switch to when we
+  // want to not be in folder.
+  inboxFolder.createSubfolder("DisplayMessageWithFolderModesB", null);
+  dummyFolder = inboxFolder.getChildNamed("DisplayMessageWithFolderModesB");
+  make_new_sets_in_folder(folder, [{count: 5}]);
+  // The message itself doesn't really matter, as long as there's at least one
+  // in the inbox.  We will delete this in teardownModule because the inbox
+  // is a shared resource and it's not okay to leave stuff in there.
+  make_new_sets_in_folder(inboxFolder, [{count: 1}]);
+
+  // Create another subfolder on the top level that is not a parent of the
+  // 2 folders so that it is not visible in Favorite mode.
+  inboxFolder.server.rootFolder.createSubfolder("Inbox2", null);
+  inbox2Folder = inboxFolder.server.rootFolder.getChildNamed("Inbox2");
+
+  be_in_folder(folder);
+  msgHdr = mc.dbView.getMsgHdrAt(0);
+}
+
+/**
+ * Test that displaying a message causes a switch to the default folder mode if
+ * the folder isn't present in the current folder mode.
+ */
+function test_display_message_with_folder_not_present_in_current_folder_mode() {
+  // Make sure the folder doesn't appear in the favorite folder mode just
+  // because it was selected last before switching
+  be_in_folder(inboxFolder);
+
+  // Move to favorite folders. This folder isn't currently a favorite folder
+  mc.folderTreeView.mode = "favorite";
+  assert_folder_not_visible(folder);
+  assert_folder_not_visible(inboxFolder);
+  assert_folder_not_visible(inbox2Folder);
+
+  // Try displaying a message
+  display_message_in_folder_tab(msgHdr);
+
+  assert_folder_mode(mc.window.kDefaultMode);
+  assert_folder_selected_and_displayed(folder);
+  assert_selected_and_displayed(msgHdr);
+}
+
+/**
+ * Test that displaying a message _does not_ cause a switch to the default
+ * folder mode if the folder is present in the current folder mode.
+ */
+function test_display_message_with_folder_present_in_current_folder_mode() {
+  // Mark the folder as a favorite
+  folder.flags |= Ci.nsMsgFolderFlags.Favorite;
+  // Also mark the dummy folder as a favorite, in preparation for
+  // test_display_message_in_smart_folder_mode_works
+  dummyFolder.flags |= Ci.nsMsgFolderFlags.Favorite;
+
+  // Make sure the folder doesn't appear in the favorite folder mode just
+  // because it was selected last before switching
+  be_in_folder(inboxFolder);
+
+  // Switch to favorite folders. Check that the folder is now in the view, as is
+  // the dummy folder
+  mc.folderTreeView.mode = "favorite";
+  assert_folder_visible(folder);
+  assert_folder_visible(dummyFolder);
+  // Also their parent folder should be visible.
+  assert_folder_visible(inboxFolder);
+  // But not a sibling of their parent, which is not Favorite.
+  assert_folder_not_visible(inbox2Folder);
+
+  // Try displaying a message
+  display_message_in_folder_tab(msgHdr);
+
+  assert_folder_mode("favorite");
+  assert_folder_selected_and_displayed(folder);
+  assert_selected_and_displayed(msgHdr);
+
+  // Now unset the flags so that we don't affect later tests.
+  folder.flags &= ~Ci.nsMsgFolderFlags.Favorite;
+  dummyFolder.flags &= ~Ci.nsMsgFolderFlags.Favorite;
+}
+
+/**
+ * Test that displaying a message in smart folders mode causes the parent in the
+ * view to expand.
+ */
+function test_display_message_in_smart_folder_mode_works() {
+  // Clear the message selection, otherwise msgHdr will still be displayed and
+  // display_message_in_folder_tab(msgHdr) will be a no-op.
+  select_none();
+  mc.folderTreeView.mode = "all";
+
+  // Switch to the dummy folder, otherwise msgHdr will be in the view and the
+  // display message in folder tab logic will simply select the message without
+  // bothering to expand any folders.
+  be_in_folder(dummyFolder);
+
+  mc.folderTreeView.mode = "smart";
+
+  let rootFolder = folder.server.rootFolder;
+  // Check that the folder is actually the child of the account root
+  assert_folder_child_in_view(folder, rootFolder);
+
+  // Collapse everything
+  smartInboxFolder = get_smart_folder_named("Inbox");
+  collapse_folder(smartInboxFolder);
+  assert_folder_collapsed(smartInboxFolder);
+  collapse_folder(rootFolder);
+  assert_folder_collapsed(rootFolder);
+  assert_folder_not_visible(folder);
+
+  // Try displaying the message
+  display_message_in_folder_tab(msgHdr);
+
+  // Check that the right folders have expanded
+  assert_folder_mode("smart");
+  assert_folder_collapsed(smartInboxFolder);
+  assert_folder_expanded(rootFolder);
+  assert_folder_selected_and_displayed(folder);
+  assert_selected_and_displayed(msgHdr);
+}
+
+/**
+ * Test that displaying a message in an inbox in smart folders mode causes the
+ * message to be displayed in the smart inbox.
+ */
+function test_display_inbox_message_in_smart_folder_mode_works() {
+  be_in_folder(inboxFolder);
+  let inboxMsgHdr = mc.dbView.getMsgHdrAt(0);
+
+  // Collapse everything
+  collapse_folder(smartInboxFolder);
+  assert_folder_collapsed(smartInboxFolder);
+  assert_folder_not_visible(inboxFolder);
+  let rootFolder = folder.server.rootFolder;
+  collapse_folder(rootFolder);
+  assert_folder_collapsed(rootFolder);
+
+  // Move to a different folder
+  be_in_folder(get_smart_folder_named("Trash"));
+  assert_message_not_in_view(inboxMsgHdr);
+
+  // Try displaying the message
+  display_message_in_folder_tab(inboxMsgHdr);
+
+  // Check that nothing has expanded, and that the right folder is selected
+  assert_folder_mode("smart");
+  assert_folder_collapsed(smartInboxFolder);
+  assert_folder_collapsed(rootFolder);
+  assert_folder_selected_and_displayed(smartInboxFolder);
+  assert_selected_and_displayed(inboxMsgHdr);
+}
+
+/**
+ * Move back to the all folders mode.
+ */
+function test_switch_to_all_folders() {
+  mc.folderTreeView.mode = "all";
+  assert_folder_tree_view_row_count(10);
+}
+
+function teardownModule() {
+  // Remove our folders
+  inboxFolder.propagateDelete(folder, true, null);
+  inboxFolder.propagateDelete(dummyFolder, true, null);
+  inboxFolder.server.rootFolder.propagateDelete(inbox2Folder, true, null);
+  assert_folder_tree_view_row_count(7);
+}
new file mode 100644
--- /dev/null
+++ b/mail/test/mozmill/folder-pane/test-folder-names-in-recent-mode.js
@@ -0,0 +1,76 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Test that the folder names have account name appended when in "recent" view.
+ */
+const MODULE_NAME = "test-folder-names-in-favorite-mode";
+
+const RELATIVE_ROOT = "../shared-modules";
+const MODULE_REQUIRES = ["folder-display-helpers"];
+
+Cu.import("resource:///modules/iteratorUtils.jsm");
+
+function setupModule(module) {
+  collector.getModule("folder-display-helpers").installInto(module);
+
+  assert_folder_mode("all");
+  assert_folder_tree_view_row_count(7);
+}
+
+function test_folder_names_in_recent_view_mode() {
+  // We need 2 local accounts that have pristine folders with
+  // unmodified times, so that it does not influence the
+  // list of Recent folders. So clear out the most-recently-used time.
+  for (let acc in fixIterator(MailServices.accounts.accounts, Ci.nsIMsgAccount)) {
+    for (let fld in fixIterator(acc.incomingServer.rootFolder.subFolders,
+                                Ci.nsIMsgFolder)) {
+      fld.setStringProperty("MRUTime", "0");
+    }
+  }
+
+  let acc1 = MailServices.accounts.accounts.queryElementAt(0, Ci.nsIMsgAccount);
+  let acc2 = MailServices.accounts.accounts.queryElementAt(1, Ci.nsIMsgAccount);
+  let rootFolder1 = acc1.incomingServer.rootFolder;
+  let rootFolder2 = acc2.incomingServer.rootFolder;
+
+  // Create some test folders.
+  rootFolder1.createSubfolder("uniqueName", null);
+  rootFolder1.createSubfolder("duplicatedName", null);
+  rootFolder2.createSubfolder("duplicatedName", null);
+  let inbox2 = rootFolder2.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox);
+  inbox2.createSubfolder("duplicatedName", null);
+
+  let fUnique = rootFolder1.getChildNamed("uniqueName");
+  let fDup1 = rootFolder1.getChildNamed("duplicatedName");
+  let fDup2 = rootFolder2.getChildNamed("duplicatedName");
+  let fDup3 = inbox2.getChildNamed("duplicatedName");
+  assert_folder_tree_view_row_count(10);
+
+  // Create some messages in the folders to make them modified.
+  make_new_sets_in_folder(fUnique, [{count: 1}]);
+  make_new_sets_in_folder(fDup1, [{count: 1}]);
+  make_new_sets_in_folder(fDup2, [{count: 2}]);
+  make_new_sets_in_folder(fDup3, [{count: 3}]);
+
+  mc.window.gFolderTreeView.mode = "recent";
+
+  // Check displayed folder names. In Recent mode the folders are sorted alphabetically
+  assert_folder_at_index_as(0, "duplicatedName - Local Folders (1)");
+  assert_folder_at_index_as(1, "duplicatedName - tinderbox@foo.invalid (3)");
+  assert_folder_at_index_as(2, "duplicatedName - tinderbox@foo.invalid (2)");
+  assert_folder_at_index_as(3, "uniqueName - Local Folders (1)");
+  assert_folder_tree_view_row_count(4);
+
+  // Remove our folders to clean up.
+  rootFolder1.propagateDelete(fUnique, true, null);
+  rootFolder1.propagateDelete(fDup1, true, null);
+  rootFolder2.propagateDelete(fDup2, true, null);
+  rootFolder2.propagateDelete(fDup3, true, null);
+}
+
+function teardownModule() {
+  mc.window.gFolderTreeView.mode = "all";
+  assert_folder_tree_view_row_count(7);
+}
--- a/mail/test/mozmill/folder-pane/test-folder-pane.js
+++ b/mail/test/mozmill/folder-pane/test-folder-pane.js
@@ -3,77 +3,77 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * Tests for the folder pane, in particular the tree view. This is kept separate
  * from the main folder-display suite so that the folders created by other tests
  * there don't influence the results here.
  */
 
-var MODULE_NAME = 'test-folder-pane';
+const MODULE_NAME = 'test-folder-pane';
 
-var RELATIVE_ROOT = '../shared-modules';
-var MODULE_REQUIRES = ['folder-display-helpers'];
-
-var folderA, folderB;
+const RELATIVE_ROOT = '../shared-modules';
+const MODULE_REQUIRES = ['folder-display-helpers'];
 
 Cu.import("resource:///modules/mailServices.js");
 
 function setupModule(module) {
-  let fdh = collector.getModule('folder-display-helpers');
-  fdh.installInto(module);
+  collector.getModule('folder-display-helpers').installInto(module);
 }
 
 /**
  * Assert the Folder Pane is in All Folder mode by default.  Check that the
  * correct number of rows for accounts and folders are always shown as new
  * folders are created, expanded, and collapsed.
  */
 function test_all_folders_toggle_folder_open_state() {
   // Test that we are in All Folders mode by default
   assert_folder_mode("all");
 
+  let pop3Server = MailServices.accounts
+                    .FindServer("tinderbox", FAKE_SERVER_HOSTNAME, "pop3");
+  collapse_folder(pop3Server.rootFolder);
+  collapse_folder(MailServices.accounts.localFoldersServer.rootFolder);
+
   // All folders mode should give us only 2 rows to start
   // (tinderbox account and local folders)
   let accounts = 2;
   assert_folder_tree_view_row_count(accounts);
 
   let inbox = trash = outbox = archives = folderPaneA = 1;
   // Create archives folder - this is ugly, but essentially the same as
   // what mailWindowOverlay.js does. We can't use the built-in helper
   // method to create the folder because we need the archive flag to get
   // set before the folder added notification is sent out, which means
   // creating the folder object via RDF, setting the flag, and then
   // creating the storage, which sends the notification.
-  let pop3Server = MailServices.accounts
-                    .FindServer("tinderbox", FAKE_SERVER_HOSTNAME, "pop3");
   let rdfService = Cc['@mozilla.org/rdf/rdf-service;1']
                      .getService(Ci.nsIRDFService);
   folder = rdfService.GetResource(pop3Server.rootFolder.URI + "/Archives").
            QueryInterface(Ci.nsIMsgFolder);
   folder.setFlag(Ci.nsMsgFolderFlags.Archive);
   folder.createStorageIfMissing(null);
   // After creating Archives, account should have expanded
   // so that we should have 5 rows visible
   assert_folder_tree_view_row_count(accounts + inbox + trash +
                                     archives);
   // close the tinderbox server.
   mc.folderTreeView.toggleOpenState(0)
-  folderA = create_folder("FolderPaneA");
+  let folderA = create_folder("FolderPaneA");
   be_in_folder(folderA);
 
   // After creating our first folder we should have 6 rows visible
   assert_folder_tree_view_row_count(accounts + inbox + trash + outbox +
                                     folderPaneA);
 
   let oneFolderCount = mc.folderTreeView.rowCount;
 
   // This makes sure the folder can be toggled
   folderA.createSubfolder("FolderPaneB", null);
-  folderB = folderA.getChildNamed("FolderPaneB");
+  let folderB = folderA.getChildNamed("FolderPaneB");
   // Enter folderB, then enter folderA. This makes sure that folderA is not
   // collapsed.
   enter_folder(folderB);
   enter_folder(folderA);
 
   // At this point folderA should be open, so the view should have one more
   // item than before (FolderPaneB).
   assert_folder_tree_view_row_count(oneFolderCount + 1);
@@ -85,9 +85,15 @@ function test_all_folders_toggle_folder_
   // folderA should be collapsed so we are back to the original count
   assert_folder_tree_view_row_count(oneFolderCount);
 
   // Toggle it back to open
   mc.folderTreeView.toggleOpenState(index);
 
   // folderB should be visible again
   assert_folder_tree_view_row_count(oneFolderCount + 1);
+
+  // Clean up
+  expand_folder(pop3Server.rootFolder);
+  folder.clearFlag(Ci.nsMsgFolderFlags.Archive);
+  pop3Server.rootFolder.propagateDelete(folder, true, null);
+  MailServices.accounts.localFoldersServer.rootFolder.propagateDelete(folderA, true, null);
 }
--- a/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ -463,17 +463,17 @@ function enter_folder(aFolder) {
 
   // this selection event may not be synchronous...
   mc.folderTreeView.selectFolder(aFolder);
   // ... so wait until it goes through by waiting on the displayedFolder...
   function isDisplayedFolder() {
     return mc.folderDisplay.displayedFolder == aFolder;
   }
   utils.waitFor(isDisplayedFolder,
-                "Timeout trying to enter folder" + aFolder.URI);
+                "Timeout trying to enter folder " + aFolder.URI);
 
   wait_for_all_messages_to_load();
 
   // and drain the event queue
   controller.sleep(0);
 }
 
 /**