Bug 500657 -- "open message from Search Messages results does not work well". Fix opening messages from a Search Messages window and add tests. r=asuth
authorSiddharth Agarwal <sid.bugzilla@gmail.com>
Wed, 01 Jul 2009 21:34:03 +0530
changeset 2987 4e87cc8301ee71a87c4db84b1bbf8147eb95264e
parent 2986 141d4eed12c4c78ca8e4af798e487b18a2c17e92
child 2988 78a7e4a8a2b91ce64b0d38fbdd37e96def2f7c1a
push id2425
push usersid.bugzilla@gmail.com
push dateWed, 01 Jul 2009 16:07:35 +0000
treeherdercomm-central@4e87cc8301ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs500657
Bug 500657 -- "open message from Search Messages results does not work well". Fix opening messages from a Search Messages window and add tests. r=asuth
mail/base/content/mailWindowOverlay.js
mail/base/content/messageWindow.js
mail/base/modules/MailUtils.js
mail/test/mozmill/folder-display/test-opening-messages.js
mail/test/mozmill/search-window/test-search-window.js
mail/test/mozmill/shared-modules/test-folder-display-helpers.js
mail/test/mozmill/shared-modules/test-window-helpers.js
--- a/mail/base/content/mailWindowOverlay.js
+++ b/mail/base/content/mailWindowOverlay.js
@@ -2255,50 +2255,23 @@ function MsgOpenSelectedMessages()
 {
   // Toggle message body (rss summary) and content-base url in message
   // pane per pref, otherwise open summary or web page in new window.
   if (gFolderDisplay.selectedMessageIsFeed && GetFeedOpenHandler() == 2) {
     FeedSetContentViewToggle();
     return;
   }
 
-  let selectedMessages = gFolderDisplay.selectedMessages;
-  let numMessages = selectedMessages.length;
-
-  let openMessageBehavior = gPrefBranch.getIntPref("mail.openMessageBehavior");
-  if (openMessageBehavior == MailConsts.OpenMessageBehavior.NEW_TAB) {
-    // Open all the tabs in the background, except for the last one
-    let tabmail = document.getElementById("tabmail");
-    for (let i = 0; i < numMessages; i++)
-      tabmail.openTab("message", {msgHdr: selectedMessages[i],
-          viewWrapperToClone: gFolderDisplay.view,
-          background: (i < (numMessages - 1))});
-    return;
-  }
-
-  if (openMessageBehavior == MailConsts.OpenMessageBehavior.EXISTING_WINDOW &&
-      numMessages == 1 && MailUtils.openMessageInExistingWindow(
-          gFolderDisplay.selectedMessage, gFolderDisplay.view))
-    return;
-
-  var openWindowWarning = gPrefBranch.getIntPref("mailnews.open_window_warning");
-  if ((openWindowWarning > 1) && (numMessages >= openWindowWarning)) {
-    if (!gMessengerBundle)
-        gMessengerBundle = document.getElementById("bundle_messenger");
-    var title = gMessengerBundle.getString("openWindowWarningTitle");
-    var text = gMessengerBundle.getFormattedString("openWindowWarningText", [numMessages]);
-    var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
-                                  .getService(Components.interfaces.nsIPromptService);
-    if (!promptService.confirm(window, title, text))
-      return;
-  }
-
-  for (var i = 0; i < numMessages; i++) {
-    MsgOpenNewWindowForMessage(selectedMessages[i]);
-  }
+  // This is somewhat evil. If we're in a 3pane window, we'd have a tabmail
+  // element and would pass it in here, ensuring that if we open tabs, we use
+  // this tabmail to open them. If we aren't, then we wouldn't, so
+  // displayMessages would look for a 3pane window and open tabs there.
+  MailUtils.displayMessages(gFolderDisplay.selectedMessages,
+                            gFolderDisplay.view,
+                            document.getElementById("tabmail"));
 }
 
 function MsgOpenFromFile()
 {
   const nsIFilePicker = Components.interfaces.nsIFilePicker;
   var fp = Components.classes["@mozilla.org/filepicker;1"]
                      .createInstance(nsIFilePicker);
 
--- a/mail/base/content/messageWindow.js
+++ b/mail/base/content/messageWindow.js
@@ -389,33 +389,47 @@ function actuallyLoadMessage() {
    *
    * We clone views when possible for:
    * - Consistency of navigation within the message display.  Users would find
    *   it odd if they showed a message from a cross-folder view but ended up
    *   navigating around the message's actual folder.
    * - Efficiency.  It's faster to clone a view than open a new one.
    *
    * Our argument idioms for the use cases are thus:
-   * 1) [A Message URI] where the URI is an nsIURL corresponding to a message
+   * 1) [{msgHdr: A message header, viewWrapperToClone: (optional) a view
+   *    wrapper to clone}]
+   * 2) [A Message header, (optional) the origin DBViewWraper]
+   * 3) [A Message URI] where the URI is an nsIURL corresponding to a message
    *     on disk or that is an attachment part on another message.
-   * 2) [A Message header, (optional) the origin DBViewWraper]
    *
    * Our original set of arguments, in case these get passed in and you're
    *  wondering why we explode, was:
    *   0: A message URI, string or nsIURI.
    *   1: A folder URI.  If arg 0 was an nsIURI, it may have had a folder attribute.
    *   2: The nsIMsgDBView used to open us.
    */
   if (window.arguments && window.arguments.length)
   {
-    // message header?
-    if (window.arguments[0] instanceof Components.interfaces.nsIMsgDBHdr) {
-      let msgHdr = window.arguments[0];
-      let originViewWrapper = window.arguments.length > 1 ?
+    let msgHdr = null, originViewWrapper = null;
+    // message header as an object?
+    if ("wrappedJSObject" in window.arguments[0]) {
+      let hdrObject = window.arguments[0].wrappedJSObject;
+      msgHdr = hdrObject.msgHdr;
+      if ("viewWrapperToClone" in hdrObject)
+        originViewWrapper = hdrObject.viewWrapperToClone;
+    }
+    // message header as a separate param?
+    else if (window.arguments[0] instanceof Components.interfaces.nsIMsgDBHdr) {
+      msgHdr = window.arguments[0];
+      originViewWrapper = window.arguments.length > 1 ?
         window.arguments[1] : null;
+    }
+
+    // this is a message header, so show it
+    if (msgHdr) {
       if (originViewWrapper)
         gFolderDisplay.cloneView(originViewWrapper);
       else
         gFolderDisplay.show(msgHdr.folder);
       gFolderDisplay.selectMessage(msgHdr);
     }
     // it must be a URI for a message lacking a backing header
     else {
--- a/mail/base/modules/MailUtils.js
+++ b/mail/base/modules/MailUtils.js
@@ -124,62 +124,108 @@ var MailUtils =
     else {
       return null;
     }
 
     return folder;
   },
 
   /**
-   * Displays this message header in a new tab, a new window or an existing
+   * Display this message header in a new tab, a new window or an existing
    * window, depending on the preference and whether a 3pane or standalone
    * window is already open. This function should be called when you'd like to
    * display a message to the user according to the pref set.
    *
+   * @note Do not use this if you want to open multiple messages at once. Use
+   *       |displayMessages| instead.
+   *
    * @param aMsgHdr the message header to display
+   * @param [aViewWrapperToClone] a view wrapper to clone. If null or not
+   *                              given, the message header's folder's default
+   *                              view will be used
+   * @param [aTabmail] a tabmail element to use in case we need to open tabs.
+   *                   If null or not given:
+   *                   - if one or more 3pane windows are open, the most recent
+   *                     one's tabmail is used
+   *                   - if no 3pane windows are open, a standalone window is
+   *                     opened instead of a tab
    */
-  displayMessage: function MailUtils_displayMessage(aMsgHdr) {
+  displayMessage: function MailUtils_displayMessage(aMsgHdr,
+                      aViewWrapperToClone, aTabmail) {
+    this.displayMessages([aMsgHdr], aViewWrapperToClone, aTabmail);
+  },
+
+  /**
+   * Display these message headers in new tabs, new windows or existing
+   * windows, depending on the preference, the number of messages, and whether
+   * a 3pane or standalone window is already open. This function should be
+   * called when you'd like to display multiple messages to the user according
+   * to the pref set.
+   *
+   * @param aMsgHdrs an array containing the message headers to display. The
+   *                 array should contain at least one message header
+   * @param [aViewWrapperToClone] a DB view wrapper to clone for each of the
+   *                              tabs or windows
+   * @param [aTabmail] a tabmail element to use in case we need to open tabs.
+   *                   If given, the window containing the tabmail is assumed
+   *                   to be in front. If null or not given:
+   *                   - if one or more 3pane windows are open, the most recent
+   *                     one's tabmail is used, and the window is brought to the
+   *                     front
+   *                   - if no 3pane windows are open, standalone windows are
+   *                     opened instead of tabs
+   */
+  displayMessages: function MailUtils_displayMessages(aMsgHdrs,
+                       aViewWrapperToClone, aTabmail) {
     let openMessageBehavior = this._prefBranch.getIntPref(
                                   "mail.openMessageBehavior");
 
     if (openMessageBehavior == MC.OpenMessageBehavior.NEW_WINDOW) {
-      this.openMessageInNewWindow(aMsgHdr);
+      this.openMessagesInNewWindows(aMsgHdrs, aViewWrapperToClone);
     }
     else if (openMessageBehavior == MC.OpenMessageBehavior.EXISTING_WINDOW) {
-      // Try reusing an existing window. If we can't, fall back to opening a
-      // new window
-      if (!this.openMessageInExistingWindow(aMsgHdr))
-        this.openMessageInNewWindow(aMsgHdr);
+      // Try reusing an existing window. If we can't, fall back to opening new
+      // windows
+      if (aMsgHdrs.length > 1 || !this.openMessageInExistingWindow(aMsgHdrs[0]))
+        this.openMessagesInNewWindows(aMsgHdrs, aViewWrapperToClone);
     }
     else if (openMessageBehavior == MC.OpenMessageBehavior.NEW_TAB) {
-      // Try opening a new tab in a 3pane window. If we can't, fall back to
-      // opening a new window
-      let windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"]
-                             .getService(Ci.nsIWindowMediator);
-      let mail3PaneWindow = windowMediator.getMostRecentWindow("mail:3pane");
-      // Do this directly instead of calling MsgOpenNewTabForMessage, because
-      // that passes in gFolderDisplay.view to be cloned, and we don't want
-      // that
-      if (mail3PaneWindow) {
-        mail3PaneWindow.document.getElementById("tabmail").openTab("message",
-            {msgHdr: aMsgHdr, background: false});
-        mail3PaneWindow.focus();
+      let mail3PaneWindow = null;
+      if (!aTabmail) {
+        // Try opening new tabs in a 3pane window
+        let windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"]
+                               .getService(Ci.nsIWindowMediator);
+        mail3PaneWindow = windowMediator.getMostRecentWindow("mail:3pane");
+        if (mail3PaneWindow)
+          aTabmail = mail3PaneWindow.document.getElementById("tabmail");
+      }
+      
+      if (aTabmail) {
+        for each (let [i, msgHdr] in Iterator(aMsgHdrs))
+          // Open all the tabs in the background, except for the last one
+          aTabmail.openTab("message", {msgHdr: msgHdr,
+              viewWrapperToClone: aViewWrapperToClone,
+              background: (i < (aMsgHdrs.length - 1))});
+
+        if (mail3PaneWindow)
+          mail3PaneWindow.focus();
       }
       else {
-        this.openMessageInNewWindow(aMsgHdr);
+        // We still haven't found a tabmail, so we'll need to open new windows
+        this.openMessagesInNewWindows(aMsgHdrs, aViewWrapperToClone);
       }
     }
   },
 
   /**
    * Show this message in an existing window.
    *
    * @param aMsgHdr the message header to display
-   * @param aViewWrapperToClone [optional] a DB view wrapper to clone for the
-   *                            message window
+   * @param [aViewWrapperToClone] a DB view wrapper to clone for the message
+   *                              window
    * @returns true if an existing window was found and the message header was
    *          displayed, false otherwise
    */
   openMessageInExistingWindow:
       function MailUtils_openMessageInExistingWindow(aMsgHdr,
                                                      aViewWrapperToClone) {
     let windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"]
                            .getService(Ci.nsIWindowMediator);
@@ -190,17 +236,57 @@ var MailUtils =
     }
     return false;
   },
 
   /**
    * Open a new standalone message window with this header.
    *
    * @param aMsgHdr the message header to display
+   * @param [aViewWrapperToClone] a DB view wrapper to clone for the message
+   *                              window
    */
-  openMessageInNewWindow: function MailUtils_openMessageInNewWindow(aMsgHdr) {
+  openMessageInNewWindow:
+      function MailUtils_openMessageInNewWindow(aMsgHdr, aViewWrapperToClone) {
+    // It sucks that we have to go through XPCOM for this
+    let args = {msgHdr: aMsgHdr, viewWrapperToClone: aViewWrapperToClone};
+    args.wrappedJSObject = args;
+
     let windowWatcher = Cc["@mozilla.org/embedcomp/window-watcher;1"]
                           .getService(Ci.nsIWindowWatcher);
     windowWatcher.openWindow(null,
         "chrome://messenger/content/messageWindow.xul", "_blank",
-        "all,chrome,dialog=no,status,toolbar", aMsgHdr);
+        "all,chrome,dialog=no,status,toolbar", args);
+  },
+
+  /**
+   * Open new standalone message windows for these headers. This will prompt
+   * for confirmation if the number of windows to be opened is greater than the
+   * value of the mailnews.open_window_warning preference.
+   *
+   * @param aMsgHdrs an array containing the message headers to display
+   * @param [aViewWrapperToClone] a DB view wrapper to clone for each message
+   *                              window
+   */
+   openMessagesInNewWindows:
+       function MailUtils_openMessagesInNewWindows(aMsgHdrs,
+                                                   aViewWrapperToClone) {
+    let openWindowWarning = this._prefBranch.getIntPref(
+                                "mailnews.open_window_warning");
+    let numMessages = aMsgHdrs.length;
+
+    if ((openWindowWarning > 1) && (numMessages >= openWindowWarning)) {
+      let bundle = Cc["@mozilla.org/intl/stringbundle;1"]
+                     .getService(Ci.nsIStringBundleService).createBundle(
+                         "chrome://messenger/locale/messenger.properties");
+
+      let title = bundle.getString("openWindowWarningTitle");
+      let message = bundle.getFormattedString("openWindowWarningText", [numMessages]);
+      let promptService = Cc["@mozilla.org/embedcomp/prompt-service;1"]
+                            .getService(Ci.nsIPromptService);
+      if (!promptService.confirm(null, title, text))
+        return;
+    }
+
+    for each (let [, msgHdr] in Iterator(aMsgHdrs))
+      this.openMessageInNewWindow(msgHdr, aViewWrapperToClone);
   }
 };
--- a/mail/test/mozmill/folder-display/test-opening-messages.js
+++ b/mail/test/mozmill/folder-display/test-opening-messages.js
@@ -76,16 +76,18 @@ function test_open_single_message_in_tab
   set_open_message_behavior("NEW_TAB");
   let folderTab = mc.tabmail.currentTabInfo;
   let preCount = mc.tabmail.tabContainer.childNodes.length;
   be_in_folder(folder);
   // Select one message
   let msgHdr = select_click_row(1);
   // Open it
   open_selected_message();
+  // This is going to trigger a message display in the main 3pane window
+  wait_for_message_display_completion(mc);
   // Check that the tab count has increased by 1
   assert_number_of_tabs_open(preCount + 1);
   // Check that the currently displayed tab is a message tab (i.e. our newly
   // opened tab is in the foreground)
   assert_tab_mode_name(null, "message");
   // Check that the message header displayed is the right one
   assert_selected_and_displayed(msgHdr);
   // Clean up, close the tab
@@ -103,16 +105,18 @@ function test_open_multiple_messages_in_
   let preCount = mc.tabmail.tabContainer.childNodes.length;
   be_in_folder(folder);
 
   // Select a bunch of messages
   select_click_row(1);
   let selectedMessages = select_shift_click_row(NUM_MESSAGES_TO_OPEN);
   // Open them
   open_selected_messages();
+  // This is going to trigger a message display in the main 3pane window
+  wait_for_message_display_completion(mc);
   // Check that the tab count has increased by the correct number
   assert_number_of_tabs_open(preCount + NUM_MESSAGES_TO_OPEN);
   // Check that the currently displayed tab is a message tab (i.e. one of our
   // newly opened tabs is in the foreground)
   assert_tab_mode_name(null, "message");
 
   // Now check whether each of the NUM_MESSAGES_TO_OPEN tabs has the correct
   // title
--- a/mail/test/mozmill/search-window/test-search-window.js
+++ b/mail/test/mozmill/search-window/test-search-window.js
@@ -48,16 +48,19 @@ function setupModule(module) {
   let fdh = collector.getModule('folder-display-helpers');
   fdh.installInto(module);
   let wh = collector.getModule('window-helpers');
   wh.installInto(module);
 }
 
 var folder, setFoo, setBar, setFooBar;
 
+// Number of messages to open for multi-message tests
+const NUM_MESSAGES_TO_OPEN = 5;
+
 /**
  * Create some messages that our constraint below will satisfy
  */
 function test_create_messages() {
   folder = create_folder("SearchWindowA");
   [setFoo, setBar, setFooBar] =
     make_new_sets_in_folder(folder, [{subject: "foo"}, {subject: "bar"},
                                      {subject: "foo bar"}]);
@@ -127,16 +130,132 @@ function test_go_search() {
   //  no windowtype, id: "virtualFolderPropertiesDialog")
   plan_for_modal_dialog("mailnews:virtualFolderProperties",
                         subtest_save_search);
   swc.click(swc.eid("saveAsVFButton"));
   wait_for_modal_dialog("mailnews:virtualFolderProperties");
 }
 
 /**
+ * Test opening a single search result in a new tab.
+ */
+function test_open_single_search_result_in_tab() {
+  swc.window.focus();
+  set_open_message_behavior("NEW_TAB");
+  let folderTab = mc.tabmail.currentTabInfo;
+  let preCount = mc.tabmail.tabContainer.childNodes.length;
+
+  // Select one message
+  let msgHdr = select_click_row(1, swc);
+  // Open the selected message
+  open_selected_message(swc);
+  // This is going to trigger a message display in the main 3pane window
+  wait_for_message_display_completion(mc);
+  // Check that the tab count has increased by 1
+  assert_number_of_tabs_open(preCount + 1);
+  // Check that the currently displayed tab is a message tab (i.e. our newly
+  // opened tab is in the foreground)
+  assert_tab_mode_name(null, "message");
+  // Check that the message header displayed is the right one
+  assert_selected_and_displayed(msgHdr);
+  // Clean up, close the tab
+  close_tab(mc.tabmail.currentTabInfo);
+  switch_tab(folderTab);
+  reset_open_message_behavior();
+}
+
+/**
+ * Test opening multiple search results in new tabs.
+ */
+function test_open_multiple_search_results_in_new_tabs() {
+  swc.window.focus();
+  set_open_message_behavior("NEW_TAB");
+  let folderTab = mc.tabmail.currentTabInfo;
+  let preCount = mc.tabmail.tabContainer.childNodes.length;
+
+  // Select a bunch of messages
+  select_click_row(1, swc);
+  let selectedMessages = select_shift_click_row(NUM_MESSAGES_TO_OPEN, swc);
+  // Open them
+  open_selected_messages(swc);
+  // This is going to trigger a message display in the main 3pane window
+  wait_for_message_display_completion(mc);
+  // Check that the tab count has increased by the correct number
+  assert_number_of_tabs_open(preCount + NUM_MESSAGES_TO_OPEN);
+  // Check that the currently displayed tab is a message tab (i.e. one of our
+  // newly opened tabs is in the foreground)
+  assert_tab_mode_name(null, "message");
+
+  // Now check whether each of the NUM_MESSAGES_TO_OPEN tabs has the correct
+  // title
+  for (let i = 0; i < NUM_MESSAGES_TO_OPEN; i++)
+    assert_tab_titled_from(mc.tabmail.tabInfo[preCount + i],
+                           selectedMessages[i]);
+
+  // Check whether each tab has the correct message, then close it to load the
+  // previous tab.
+  for (let i = 0; i < NUM_MESSAGES_TO_OPEN; i++) {
+    assert_selected_and_displayed(selectedMessages.pop());
+    close_tab(mc.tabmail.currentTabInfo);
+  }
+  switch_tab(folderTab);
+  reset_open_message_behavior();
+}
+
+/**
+ * Test opening a search result in a new window.
+ */
+function test_open_search_result_in_new_window() {
+  swc.window.focus();
+  set_open_message_behavior("NEW_WINDOW");
+
+  // Select a message
+  let msgHdr = select_click_row(1, swc);
+
+  plan_for_new_window("mail:messageWindow");
+  // Open it
+  open_selected_message(swc);
+  let msgc = wait_for_new_window("mail:messageWindow");
+  wait_for_message_display_completion(msgc, true);
+
+  assert_selected_and_displayed(msgc, msgHdr);
+  // Clean up, close the window
+  close_message_window(msgc);
+  reset_open_message_behavior();
+}
+
+/**
+ * Test reusing an existing window to open another search result.
+ */
+function test_open_search_result_in_existing_window() {
+  swc.window.focus();
+  set_open_message_behavior("EXISTING_WINDOW");
+
+  // Open up a window
+  select_click_row(1, swc);
+  plan_for_new_window("mail:messageWindow");
+  open_selected_message(swc);
+  let msgc = wait_for_new_window("mail:messageWindow");
+  wait_for_message_display_completion(msgc, true);
+
+  // Select another message and open it
+  let msgHdr = select_click_row(2, swc);
+  open_selected_message(swc);
+  // We don't need to pass true here, as open_selected_message should have
+  // started off the load before returning.
+  wait_for_message_display_completion(msgc);
+
+  // Check if our old window displays the message
+  assert_selected_and_displayed(msgc, msgHdr);
+  // Clean up, close the window
+  close_message_window(msgc);
+  reset_open_message_behavior();
+}
+
+/**
  * Save the search, making sure the constraints propagated.
  */
 function subtest_save_search(savc) {
   // - make sure our constraint propagated
   // The query constraints are displayed using the same widgets (and code) that
   //  we used to enter them, so it's very similar to check.
   let searchVal0 = savc.aid("searchVal0", {crazyDeck: 0});
   savc.assertNode(searchVal0);
@@ -152,16 +271,17 @@ function subtest_save_search(savc) {
 
   // - save it!
   // this will close the dialog, which wait_for_modal_dialog is making sure
   //  happens.
   savc.window.onOK();
 }
 
 function test_close_search_window() {
+  swc.window.focus();
   // now close the search window
   plan_for_window_close(swc);
   swc.keypress(null, "VK_ESCAPE", {});
   wait_for_window_close(swc);
   swc = null;
 }
 
 /**
--- a/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ -245,22 +245,30 @@ function open_folder_in_new_tab(aFolder)
   mc.tabmail.openTab("folder", {folder: aFolder});
   wait_for_all_messages_to_load();
   return mc.tabmail.currentTabInfo;
 }
 
 /**
  * Open the selected message(s) by pressing Enter. The mail.openMessageBehavior
  * pref is supposed to determine how the messages are opened.
+ *
+ * Since we don't know where this is going to trigger a message load, you're
+ * going to have to wait for message display completion yourself.
+ *
+ * @param aController The controller in whose context to do this, defaults to
+ *     |mc| if omitted.
  */
-function open_selected_messages() {
+function open_selected_messages(aController) {
+  if (aController === undefined)
+    aController = mc;
   // Focus the thread tree
-  mc.threadTree.focus();
+  aController.threadTree.focus();
   // Open whatever's selected
-  press_enter();
+  press_enter(aController);
 }
 
 var open_selected_message = open_selected_messages;
 
 /**
  * Create a new tab displaying the currently selected message, making that tab
  *  the current tab.  We block until the message finishes loading.
  *
@@ -423,38 +431,50 @@ function close_message_window(aControlle
  */
 function select_none() {
   wait_for_message_display_completion();
   mc.dbView.selection.clearSelection();
   // give the event queue a chance to drain...
   controller.sleep(0);
 }
 
-function _normalize_view_index(aViewIndex) {
+function _normalize_view_index(aViewIndex, aController) {
+  if (aController === undefined)
+    aController = mc;
   if (aViewIndex < 0)
-    return mc.dbView.QueryInterface(Ci.nsITreeView).rowCount + aViewIndex;
+    return aController.dbView.QueryInterface(Ci.nsITreeView).rowCount +
+      aViewIndex;
   return aViewIndex;
 }
 
 /**
  * Pretend we are clicking on a row with our mouse.
  *
  * @param aViewIndex If >= 0, the view index provided, if < 0, a reference to
  *     a view index counting from the last row in the tree.  -1 indicates the
  *     last message in the tree, -2 the second to last, etc.
+ * @param aController The controller in whose context to do this, defaults to
+ *     |mc| if omitted.
  *
  * @return The message header selected.
  */
-function select_click_row(aViewIndex) {
-  wait_for_message_display_completion();
-  aViewIndex = _normalize_view_index(aViewIndex);
+function select_click_row(aViewIndex, aController) {
+  if (aController === undefined)
+    aController = mc;
+  let hasMessageDisplay = "messageDisplay" in aController;
+  if (hasMessageDisplay)
+    wait_for_message_display_completion(aController);
+  aViewIndex = _normalize_view_index(aViewIndex, aController);
+
   // this should set the current index as well as setting the selection.
-  mc.dbView.selection.select(aViewIndex);
-  wait_for_message_display_completion(mc, mc.messageDisplay.visible);
-  return mc.dbView.getMsgHdrAt(aViewIndex);
+  aController.dbView.selection.select(aViewIndex);
+  if (hasMessageDisplay)
+    wait_for_message_display_completion(aController,
+                                        aController.messageDisplay.visible);
+  return aController.dbView.getMsgHdrAt(aViewIndex);
 }
 
 /**
  * Pretend we are toggling the thread specified by a row.
  *
  * @param aViewIndex If >= 0, the view index provided, if < 0, a reference to
  *     a view index counting from the last row in the tree.  -1 indicates the
  *     last message in the tree, -2 the second to last, etc.
@@ -495,32 +515,40 @@ function select_control_click_row(aViewI
 
 /**
  * Pretend we are clicking on a row with our mouse with the shift key pressed,
  *  adding all the messages between the shift pivot and the shift selected row.
  *
  * @param aViewIndex If >= 0, the view index provided, if < 0, a reference to
  *     a view index counting from the last row in the tree.  -1 indicates the
  *     last message in the tree, -2 the second to last, etc.
+ * @param aController The controller in whose context to do this, defaults to
+ *     |mc| if omitted.
  *
  * @return The message headers for all messages that are now selected.
  */
-function select_shift_click_row(aViewIndex) {
-  wait_for_message_display_completion();
-  aViewIndex = _normalize_view_index(aViewIndex);
+function select_shift_click_row(aViewIndex, aController) {
+  if (aController === undefined)
+    aController = mc;
+  let hasMessageDisplay = "messageDisplay" in aController;
+  if (hasMessageDisplay)
+    wait_for_message_display_completion(aController);
+  aViewIndex = _normalize_view_index(aViewIndex, aController);
+
   // Passing -1 as the start range checks the shift-pivot, which should be -1,
   //  so it should fall over to the current index, which is what we want.  It
   //  will then set the shift-pivot to the previously-current-index and update
   //  the current index to be what we shift-clicked on.  All matches user
   //  interaction.
-  mc.dbView.selection.rangedSelect(-1, aViewIndex, false);
+  aController.dbView.selection.rangedSelect(-1, aViewIndex, false);
   // give the event queue a chance to drain...
   controller.sleep(0);
-  wait_for_message_display_completion();
-  return mc.folderDisplay.selectedMessages;
+  if (hasMessageDisplay)
+    wait_for_message_display_completion(aController);
+  return aController.folderDisplay.selectedMessages;
 }
 
 /**
  * Helper function to click on a row with a given button.
  */
 function _row_click_helper(aViewIndex, aButton) {
   let treeBox = mc.threadTree.treeBoxObject;
   // very important, gotta be able to see the row
@@ -616,29 +644,31 @@ function press_delete(aController) {
                                  "DeleteOrMoveMsgFailed");
   aController.keypress(aController == mc ? mc.eThreadTree : null,
                        "VK_DELETE", {});
   wait_for_folder_events();
 }
 
 /**
  * Pretend we are pressing the Enter key, triggering opening selected messages.
+ * Note that since we don't know where this is going to trigger a message load,
+ * you're going to have to wait for message display completion yourself.
  *
  * @param aController The controller in whose context to do this, defaults to
  *     |mc| if omitted.
  */
 function press_enter(aController) {
   if (aController === undefined)
     aController = mc;
   // if something is loading, make sure it finishes loading...
-  wait_for_message_display_completion(aController);
+  if ("messageDisplay" in aController)
+    wait_for_message_display_completion(aController);
   aController.keypress(aController == mc ? mc.eThreadTree : null,
                        "VK_RETURN", {});
-  // this is always going to cause a message load, so wait for that
-  wait_for_message_display_completion(aController);
+  // The caller's going to have to wait for message display completion
 }
 
 /**
  * Wait for the |folderDisplay| on aController (defaults to mc if omitted) to
  *  finish loading.  This generally only matters for folders that have an active
  *  search.
  * This method is generally called automatically most of the time, and you
  *  should not need to call it yourself unless you are operating outside the
--- a/mail/test/mozmill/shared-modules/test-window-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-window-helpers.js
@@ -710,18 +710,26 @@ var PerWindowTypeAugmentations = {
       },
     },
   },
 
   /**
    * The search window, via control-shift-F.
    */
   "mailnews:search": {
+    elementsToExpose: {
+      threadTree: "threadTree",
+    },
     globalsToExposeAtStartup: {
       folderDisplay: "gFolderDisplay",
+    },
+    getters: {
+      dbView: function () {
+        return this.folderDisplay.view.dbView;
+      }
     }
   }
 };
 
 function _augment_helper(aController, aAugmentDef) {
   if (aAugmentDef.elementsToExpose) {
     for each (let [key, value] in Iterator(aAugmentDef.elementsToExpose)) {
       aController[key] = aController.window.document.getElementById(value);