Bug 871266 - properly close customize dialog in test_customize_toolbar_doesnt_double_get_mail_menu. r=mconley
authoraceman <acelists@atlas.sk>
Mon, 10 Jun 2013 12:28:59 +0100
changeset 15710 a7f0ffd07bb2387392b84c7a1b5e88bc06e3e8ca
parent 15709 3c7cb0989feffc69e541e07e78a24814a65c8288
child 15711 a2966564a3c222e6172af35eefe796fcfb182bad
push id942
push userbugzilla@standard8.plus.com
push dateMon, 05 Aug 2013 19:15:38 +0000
treeherdercomm-beta@0e1a1c4a9f0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs871266
Bug 871266 - properly close customize dialog in test_customize_toolbar_doesnt_double_get_mail_menu. r=mconley
mail/base/content/customizeToolbarOverlay.xul
mail/base/content/mailCore.js
mail/base/content/mailWindowOverlay.xul
mail/test/mozmill/folder-widget/test-message-filters.js
--- a/mail/base/content/customizeToolbarOverlay.xul
+++ b/mail/base/content/customizeToolbarOverlay.xul
@@ -14,13 +14,14 @@
 
   <script type="application/javascript"
           src="chrome://messenger/content/mailCore.js"/>
 
   <menulist id="modelist"
             oncommand="overlayUpdateToolbarMode(this.value, 'mail-toolbox');"/>
 
   <window id="CustomizeToolbarWindow"
+          windowtype="mailnews:customizeToolbar"
           onload="overlayOnLoad();">
     <data id="iconsBesideText.label"
           value="&iconsBesideText.label;"/>
   </window>
 </overlay>
--- a/mail/base/content/mailCore.js
+++ b/mail/base/content/mailCore.js
@@ -211,23 +211,38 @@ function MailToolboxCustomizeDone(aEvent
 
   let toolbox = document.querySelector('[doCustomization="true"]');
   if (toolbox) {
     toolbox.removeAttribute("doCustomization");
 
     // The GetMail button is stuck in a strange state right now, since the
     // customization wrapping preserves its children, but not its initialized
     // state. Fix that here.
+    // That is also true for the File -> "Get new messages for" menuitems in both
+    // menus (old and new App menu).
+    // TODO: try to fix folderWidgets.xml to not do this.
+    // See Bug 520457 and Bug 534448.
     // Fix Bug 565045: Only treat "Get Message Button" if it is in our toolbox
-    let popup = toolbox.querySelector("#button-getMsgPopup");
-    if (popup) {
-      // We can't use _teardown here, because it'll remove the Get All menuitem
-      let sep = toolbox.querySelector("#button-getAllNewMsgSeparator");
-      while (popup.lastChild != sep)
-        popup.removeChild(popup.lastChild);
+    for (let popup of [ toolbox.querySelector("#button-getMsgPopup"),
+                        document.getElementById("menu_getAllNewMsgPopup"),
+                        document.getElementById("appmenu_getAllNewMsgPopup") ])
+    {
+      if (!popup)
+        continue;
+
+      // .teardown() is only available here if the menu has its frame
+      // otherwise the folderWidgets.xml::folder-menupopup binding is not
+      // attached to the popup. So if it is not available, remove the items
+      // explicitly, starting the the menuseparator element.
+      if ("_teardown" in popup) {
+        popup._teardown();
+      } else {
+        while (popup.lastChild.tagName != "menuseparator")
+          popup.removeChild(popup.lastChild);
+      }
     }
   }
 }
 
 function onViewToolbarsPopupShowing(aEvent, toolboxIds, aInsertPoint)
 {
   if (!Array.isArray(toolboxIds))
     toolboxIds = [toolboxIds];
--- a/mail/base/content/mailWindowOverlay.xul
+++ b/mail/base/content/mailWindowOverlay.xul
@@ -2228,16 +2228,17 @@
                command="cmd_saveAsFile"/>
               <menuitem id="menu_saveAsTemplate" label="&saveAsTemplateCmd.label;"
                accesskey="&saveAsTemplateCmd.accesskey;"
                command="cmd_saveAsTemplate"/>
             </menupopup>
           </menu>
           <menuseparator id="fileMenuAfterSaveSeparator"/>
           <menu label="&getNewMsgForCmd.label;" accesskey="&getNewMsgForCmd.accesskey;"
+                id="menu_getAllNewMsg"
                 oncommand="MsgGetMessagesForAccount(event.target._folder)">
             <menupopup type="folder" mode="getMail" id="menu_getAllNewMsgPopup"
                        expandFolders="false">
               <menuitem id="menu_getnewmsgs_all_accounts"
                         label="&getAllNewMsgCmdPopupMenu.label;"
                         accesskey="&getAllNewMsgCmdPopupMenu.accesskey;"
                         key="key_getAllNewMessages"
                         command="cmd_getMsgsForAuthAccounts"/>
--- a/mail/test/mozmill/folder-widget/test-message-filters.js
+++ b/mail/test/mozmill/folder-widget/test-message-filters.js
@@ -94,32 +94,47 @@ function test_message_filter_shows_newsg
 /*
  * Test that customizing the toolbar doesn't lead to doubled accounts in
  * the Get Mail menu.  (bug 520457)
  */
 function test_customize_toolbar_doesnt_double_get_mail_menu()
 {
   be_in_folder(folderA);
 
-  popup = mc.eid("menu_getAllNewMsgPopup");
-  mc.assertNode(popup);
-  let menu = new elib.Elem(popup.node.parentNode);
-  // This one initializes the menuitems, but it's kinda hacky.
-  popup.node._ensureInitialized();
-  assert_equals(menu.node.itemCount, 5,
-                "Incorrect number of items for GetNewMessages before customization");
+  /**
+   * Get the getAllNewMessages menu and check the number of items.
+   */
+  function check_getAllNewMsgMenu() {
+    mc.click(mc.eid("menu_File"), 5, 5);
+    wait_for_popup_to_open(mc.e("menu_FilePopup"));
+
+    let menu = mc.eid("menu_getAllNewMsg");
+    mc.click(menu, 5, 5);
+    wait_for_popup_to_open(mc.e("menu_getAllNewMsgPopup"));
 
+    assert_equals(menu.node.itemCount, 5,
+                  "Incorrect number of items for GetNewMessages before customization");
+
+    close_popup(mc, mc.eid("menu_getAllNewMsgPopup"));
+    close_popup(mc, mc.eid("menu_FilePopup"));
+  }
+
+  check_getAllNewMsgMenu();
+
+  plan_for_new_window("mailnews:customizeToolbar");
   // Open the customization dialog.
   mc.rightClick(mc.eid("mail-bar3"));
   mc.click(mc.eid("CustomizeMailToolbar"));
 
-  let toolbox = mc.eid("mail-toolbox");
-  toolbox.node.customizeDone();
-  assert_equals(menu.node.itemCount, 5,
-                "Incorrect number of items for GetNewMessages after customization");
+  let customc = wait_for_new_window("mailnews:customizeToolbar");
+  plan_for_window_close(customc);
+  customc.click(customc.eid("donebutton"));
+  wait_for_window_close();
+
+  check_getAllNewMsgMenu();
 }
 
 /* A helper function that opens up the new filter dialog (assuming that the
  * main filters dialog is already open), creates a simple filter, and then
  * closes the dialog.
  */
 function create_simple_filter() {
   // Open the "Tools » Message Filters…" window,