Bug 1838770 - Fix Folder Properties menu item. r=mkmelin

Differential Revision: https://phabricator.services.mozilla.com/D181587

--HG--
extra : rebase_source : 6a1e6f813c28c278922ea7198186ef78fe15b1a2
extra : amend_source : afb8497867c2e6553d000f9a3b3507b5cca56271
extra : histedit_source : d87e4db1a721ec66c7097663d244a07683850aff

mail/base/content/about3Pane.js

@@ -245,6 +245,7 @@ var folderPaneContextMenu = {
     "folderPaneContext-remove": "cmd_deleteFolder",
     "folderPaneContext-rename": "cmd_renameFolder",
     "folderPaneContext-compact": "cmd_compactFolder",
+    "folderPaneContext-properties": "cmd_properties",
   },
 
   /**
@@ -358,6 +359,7 @@ var folderPaneContextMenu = {
           (isServer || canCompact) &&
           folder.isCommandEnabled("cmd_compactFolder"),
         cmd_emptyTrash: !isNNTP,
+        cmd_properties: !isServer && !FolderUtils.isSmartTagsFolder(folder),
       };
     }
     return this._commandStates[command];
@@ -479,7 +481,6 @@ var folderPaneContextMenu = {
         flags & Ci.nsMsgFolderFlags.Favorite
       );
     }
-    showItem("folderPaneContext-properties", !isServer && !isSmartTagsFolder);
     showItem("folderPaneContext-markAllFoldersRead", isServer);
 
     showItem("folderPaneContext-settings", isServer);
@@ -5995,6 +5996,11 @@ commandController.registerCallback(
   (folder = gFolder) => folderPane.emptyTrash(folder),
   () => folderPaneContextMenu.getCommandState("cmd_emptyTrash")
 );
+commandController.registerCallback(
+  "cmd_properties",
+  (folder = gFolder) => folderPane.editFolder(folder),
+  () => folderPaneContextMenu.getCommandState("cmd_properties")
+);
 
 // Delete commands, which change behaviour based on the active element.
 // Note that `document.activeElement` refers to the active element in *this*

mail/base/content/mailWindowOverlay.js

@@ -167,45 +167,41 @@ function InitEditMessagesMenu() {
   document.commandDispatcher.updateCommands("create-menu-edit");
 
   let chromeBrowser, folderTreeActive, folderIsNewsgroup;
+  let tab = document.getElementById("tabmail")?.currentTabInfo;
+  if (tab?.mode.name == "mail3PaneTab") {
+    chromeBrowser = tab.chromeBrowser;
+    folderTreeActive =
+      chromeBrowser.contentDocument.activeElement.id == "folderTree";
+    let folder = chromeBrowser.contentWindow.gFolder;
+    folderIsNewsgroup = folder?.server.type == "nntp";
+  } else if (tab?.mode.name == "mailMessageTab") {
+    chromeBrowser = tab.chromeBrowser;
+  } else {
+    chromeBrowser = document.getElementById("messageBrowser");
+  }
 
   let deleteController = getEnabledControllerForCommand("cmd_delete");
-  if (deleteController?.wrappedJSObject) {
-    // If the controller is a JS object, it must be one we've implemented,
-    // not the built-in controller for textboxes.
-    let tab = document.getElementById("tabmail")?.currentTabInfo;
-    if (tab?.mode.name == "mail3PaneTab") {
-      chromeBrowser = tab.chromeBrowser;
-      folderTreeActive =
-        chromeBrowser.contentDocument.activeElement.id == "folderTree";
-      if (folderTreeActive) {
-        let folder = chromeBrowser.contentWindow.gFolder;
-        folderIsNewsgroup = folder?.server.type == "nntp";
-      }
-    } else if (tab?.mode.name == "mailMessageTab") {
-      chromeBrowser = tab.chromeBrowser;
-    } else {
-      chromeBrowser = document.getElementById("messageBrowser");
-    }
-  }
+  // If the controller is a JS object, it must be one we've implemented,
+  // not the built-in controller for textboxes.
 
   let dbView = chromeBrowser?.contentWindow.gDBView;
   let numSelected = dbView?.numSelected;
 
   let deleteMenuItem = document.getElementById("menu_delete");
-  if (folderTreeActive) {
-    let value = folderIsNewsgroup ? "unsubscribe-newsgroup" : "delete-folder";
-    document.l10n.setAttributes(deleteMenuItem, `menu-edit-${value}`);
-  } else if (numSelected) {
+  if (deleteController?.wrappedJSObject && folderTreeActive) {
+    let value = folderIsNewsgroup
+      ? "menu-edit-unsubscribe-newsgroup"
+      : "menu-edit-delete-folder";
+    document.l10n.setAttributes(deleteMenuItem, value);
+  } else if (deleteController?.wrappedJSObject && numSelected) {
     let message = dbView?.hdrForFirstSelectedMessage;
     let value;
     if (message && message.flags & Ci.nsMsgMessageFlags.IMAPDeleted) {
-      value = "undelete-messages";
+      value = "menu-edit-undelete-messages";
     } else {
-      value = "delete-messages";
+      value = "menu-edit-delete-messages";
     }
-    document.l10n.setAttributes(deleteMenuItem, `menu-edit-${value}`, {
-      count: numSelected,
-    });
+    document.l10n.setAttributes(deleteMenuItem, value, { count: numSelected });
   } else {
     document.l10n.setAttributes(deleteMenuItem, "text-action-delete");
   }
@@ -215,6 +211,17 @@ function InitEditMessagesMenu() {
   if (!favoriteFolderMenu.hasAttribute("disabled")) {
     // TODO: Reimplement this as a command.
   }
+
+  let propertiesController = getEnabledControllerForCommand("cmd_properties");
+  let propertiesMenuItem = document.getElementById("menu_properties");
+  if (tab?.mode.name == "mail3PaneTab" && propertiesController) {
+    let value = folderIsNewsgroup
+      ? "menu-edit-newsgroup-properties"
+      : "menu-edit-folder-properties";
+    document.l10n.setAttributes(propertiesMenuItem, value);
+  } else {
+    document.l10n.setAttributes(propertiesMenuItem, "menu-edit-properties");
+  }
 }
 
 /**

mail/base/content/mainCommandSet.inc.xhtml

@@ -107,10 +107,7 @@
     <command id="cmd_cancel" oncommand="goDoCommand('cmd_cancel')"/>
     <command id="cmd_selectThread" oncommand="goDoCommand('cmd_selectThread')"/>
     <command id="cmd_selectFlagged" oncommand="goDoCommand('cmd_selectFlagged')"/>
-    <command id="cmd_properties" oncommand="goDoCommand('cmd_properties')"
-           valueNewsgroup="&folderPropsNewsgroupCmd2.label;"
-           valueFolder="&folderPropsFolderCmd2.label;"
-           valueGeneric="&folderPropsCmd2.label;"/>
+    <command id="cmd_properties" oncommand="goDoCommand('cmd_properties')"/>
     <command id="cmd_find" oncommand="goDoCommand('cmd_find')" disabled="true"/>
     <command id="cmd_findAgain" oncommand="goDoCommand('cmd_findAgain')" disabled="true"/>
     <command id="cmd_findPrevious" oncommand="goDoCommand('cmd_findPrevious')"

mail/base/content/messenger-menubar.inc.xhtml

@@ -297,8 +297,7 @@
               accesskey="&menuFavoriteFolder.accesskey;"
               checked="false"
               oncommand="ToggleFavoriteFolderFlag();"/>
-    <menuitem id="menu_properties" label="&folderPropsCmd2.label;"
-              accesskey="&folderPropsCmd.accesskey;"
+    <menuitem id="menu_properties"
               command="cmd_properties"/>
 #ifdef MAIN_WINDOW
     <menuitem id="calendar-properties-menuitem"

mail/base/test/browser/browser_editMenu.js

@@ -44,7 +44,7 @@ if (AppConstants.platform == "linux") {
 let helper = new MenuTestHelper("menu_Edit", editMenuData);
 
 let tabmail = document.getElementById("tabmail");
-let rootFolder, testFolder, testMessages;
+let rootFolder, testFolder, testMessages, virtualFolder;
 let nntpRootFolder, nntpFolder;
 let imapRootFolder, imapFolder;
 
@@ -67,6 +67,14 @@ add_setup(async function () {
   );
   testMessages = [...testFolder.messages];
 
+  rootFolder.createSubfolder("edit menu virtual", null);
+  virtualFolder = rootFolder.getChildNamed("edit menu virtual");
+  virtualFolder.setFlag(Ci.nsMsgFolderFlags.Virtual);
+  let msgDatabase = virtualFolder.msgDatabase;
+  let folderInfo = msgDatabase.dBFolderInfo;
+  folderInfo.setCharProperty("searchStr", "ALL");
+  folderInfo.setCharProperty("searchFolderUri", testFolder.URI);
+
   NNTPServer.open();
   NNTPServer.addGroup("edit.menu.newsgroup");
   let nntpAccount = MailServices.accounts.createAccount();
@@ -135,12 +143,12 @@ add_task(async function testDeleteItem() {
   // calling cmd_delete actually attempts to unsubscribe the folder.
 
   displayFolder(nntpFolder);
-  await helper.testItems({
-    menu_delete: { l10nID: "menu-edit-unsubscribe-newsgroup" },
-  });
-  let promptPromise = BrowserTestUtils.promiseAlertDialog("cancel");
-  goDoCommand("cmd_delete");
-  await promptPromise;
+  await Promise.all([
+    BrowserTestUtils.promiseAlertDialog("cancel"),
+    helper.activateItem("menu_delete", {
+      l10nID: "menu-edit-unsubscribe-newsgroup",
+    }),
+  ]);
 
   // Check that a mail account shows "Delete Folder". The account can't be
   // deleted this way so the menu item should be disabled.
@@ -158,12 +166,10 @@ add_task(async function testDeleteItem() {
   // the folder.
 
   displayFolder(testFolder);
-  await helper.testItems({
-    menu_delete: { l10nID: "menu-edit-delete-folder" },
-  });
-  promptPromise = BrowserTestUtils.promiseAlertDialog("cancel");
-  goDoCommand("cmd_delete");
-  await promptPromise;
+  await Promise.all([
+    BrowserTestUtils.promiseAlertDialog("cancel"),
+    helper.activateItem("menu_delete", { l10nID: "menu-edit-delete-folder" }),
+  ]);
   await new Promise(resolve => setTimeout(resolve));
 
   // Focus the Quick Filter bar text box and check the menu item shows "Delete".
@@ -225,19 +231,16 @@ add_task(async function testDeleteItem() {
 
   threadTree.table.body.focus();
   threadTree.selectedIndices = [0, 1, 3];
-  await helper.testItems({
-    menu_delete: {
+  await Promise.all([
+    new PromiseTestUtils.promiseFolderEvent(
+      testFolder,
+      "DeleteOrMoveMsgCompleted"
+    ),
+    helper.activateItem("menu_delete", {
       l10nID: "menu-edit-delete-messages",
       l10nArgs: { count: 3 },
-    },
-  });
-  let deleteListener = new PromiseTestUtils.promiseFolderEvent(
-    testFolder,
-    "DeleteOrMoveMsgCompleted"
-  );
-  goDoCommand("cmd_delete");
-  await deleteListener.promise;
-  await new Promise(resolve => setTimeout(resolve));
+    }),
+  ]);
 
   // Load an IMAP folder with the "just mark deleted" model. With no messages
   // selected check the menu item shows "Delete".
@@ -261,13 +264,10 @@ add_task(async function testDeleteItem() {
 
   threadTree.selectedIndex = 0;
   let message = dbView.getMsgHdrAt(0);
-  await helper.testItems({
-    menu_delete: {
-      l10nID: "menu-edit-delete-messages",
-      l10nArgs: { count: 1 },
-    },
+  await helper.activateItem("menu_delete", {
+    l10nID: "menu-edit-delete-messages",
+    l10nArgs: { count: 1 },
   });
-  goDoCommand("cmd_delete");
   // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
   await new Promise(resolve => setTimeout(resolve, 1000));
   Assert.ok(
@@ -280,13 +280,10 @@ add_task(async function testDeleteItem() {
 
   // The delete operation moved the selection, go back.
   threadTree.selectedIndex = 0;
-  await helper.testItems({
-    menu_delete: {
-      l10nID: "menu-edit-undelete-messages",
-      l10nArgs: { count: 1 },
-    },
+  await helper.activateItem("menu_delete", {
+    l10nID: "menu-edit-undelete-messages",
+    l10nArgs: { count: 1 },
   });
-  goDoCommand("cmd_delete");
   // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
   await new Promise(resolve => setTimeout(resolve, 1000));
   Assert.ok(
@@ -314,7 +311,7 @@ add_task(async function testDeleteItem() {
       l10nArgs: { count: 3 },
     },
   });
-  goDoCommand("cmd_delete");
+  await helper.activateItem("menu_delete");
   // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
   await new Promise(resolve => setTimeout(resolve, 1000));
   Assert.ok(
@@ -326,13 +323,10 @@ add_task(async function testDeleteItem() {
   // cmd_delete clears the flag on the messages.
 
   threadTree.selectedIndices = [1, 3, 5];
-  await helper.testItems({
-    menu_delete: {
-      l10nID: "menu-edit-undelete-messages",
-      l10nArgs: { count: 3 },
-    },
+  await helper.activateItem("menu_delete", {
+    l10nID: "menu-edit-undelete-messages",
+    l10nArgs: { count: 3 },
   });
-  goDoCommand("cmd_delete");
   // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
   await new Promise(r => setTimeout(r, 1000));
   Assert.ok(
@@ -349,8 +343,71 @@ add_task(async function testDeleteItem() {
       l10nArgs: { count: 3 },
     },
   });
+
+  Services.focus.focusedWindow = window;
 }).__skipMe = AppConstants.DEBUG; // Too unreliable.
 
+/**
+ * Tests the "Properties" item in the menu is enabled/disabled as expected,
+ * and has the correct label.
+ */
+add_task(async function testPropertiesItem() {
+  async function testDialog(folder, data, which = "folderProps.xhtml") {
+    await Promise.all([
+      BrowserTestUtils.promiseAlertDialog(
+        undefined,
+        `chrome://messenger/content/${which}`,
+        {
+          callback(win) {
+            Assert.ok(true, "folder properties dialog opened");
+            Assert.equal(
+              win.gMsgFolder.URI,
+              folder.URI,
+              "dialog has correct folder"
+            );
+            win.document.querySelector("dialog").getButton("cancel").click();
+          },
+        }
+      ),
+      helper.activateItem("menu_properties", data),
+    ]);
+    await SimpleTest.promiseFocus(window);
+  }
+
+  let { displayFolder } = tabmail.currentAbout3Pane;
+
+  displayFolder(rootFolder);
+  await helper.testItems({
+    menu_properties: { disabled: true, l10nID: "menu-edit-properties" },
+  });
+
+  displayFolder(testFolder);
+  await testDialog(testFolder, { l10nID: "menu-edit-folder-properties" });
+
+  displayFolder(virtualFolder);
+  await testDialog(
+    virtualFolder,
+    { l10nID: "menu-edit-folder-properties" },
+    "virtualFolderProperties.xhtml"
+  );
+
+  displayFolder(imapRootFolder);
+  await helper.testItems({
+    menu_properties: { disabled: true, l10nID: "menu-edit-properties" },
+  });
+
+  displayFolder(imapFolder);
+  await testDialog(imapFolder, { l10nID: "menu-edit-folder-properties" });
+
+  displayFolder(nntpRootFolder);
+  await helper.testItems({
+    menu_properties: { disabled: true, l10nID: "menu-edit-properties" },
+  });
+
+  displayFolder(nntpFolder);
+  await testDialog(nntpFolder, { l10nID: "menu-edit-newsgroup-properties" });
+});
+
 var NNTPServer = {
   open() {
     let { NNTP_RFC977_handler, NntpDaemon } = ChromeUtils.import(

mail/base/test/browser/head.js

@@ -92,6 +92,18 @@ class MenuTestHelper {
     this.baseData = baseData;
   }
 
+  /**
+   * Clicks on the menu and waits for it to open.
+   */
+  async openMenu() {
+    let shownPromise = BrowserTestUtils.waitForEvent(
+      this.menu.menupopup,
+      "popupshown"
+    );
+    EventUtils.synthesizeMouseAtCenter(this.menu, {});
+    await shownPromise;
+  }
+
   /**
    * Check that an item matches the expected state.
    *
@@ -177,6 +189,7 @@ class MenuTestHelper {
     }
 
     popup.hidePopup();
+    await new Promise(resolve => setTimeout(resolve));
   }
 
   /**
@@ -199,7 +212,7 @@ class MenuTestHelper {
     }
 
     // Open the menu and all submenus and check the items.
-    EventUtils.synthesizeMouseAtCenter(this.menu, {});
+    await this.openMenu();
     await this.iterate(this.menu.menupopup, data, true);
 
     // Report any unexpected items.
@@ -214,20 +227,45 @@ class MenuTestHelper {
    * @param {MenuData} data - The expected values to test against.
    */
   async testItems(data) {
-    let shownPromise = BrowserTestUtils.waitForEvent(
-      this.menu.menupopup,
-      "popupshown"
-    );
-    EventUtils.synthesizeMouseAtCenter(this.menu, {});
-    await shownPromise;
-
+    await this.openMenu();
     await this.iterate(this.menu.menupopup, data);
 
     for (let id of Object.keys(data)) {
       Assert.report(true, undefined, undefined, `extra item ${id} in data`);
     }
 
-    this.menu.menupopup.hidePopup();
+    if (this.menu.menupopup.state != "closed") {
+      let hiddenPromise = BrowserTestUtils.waitForEvent(
+        this.menu.menupopup,
+        "popuphidden"
+      );
+      this.menu.menupopup.hidePopup();
+      await hiddenPromise;
+    }
+    await new Promise(resolve => setTimeout(resolve));
+  }
+
+  /**
+   * Activates the item in the menu.
+   *
+   * @note This currently only works on top-level items.
+   * @param {string} menuItemID - The item to activate.
+   * @param {MenuData} [data] - If given, the expected state of the menu item
+   *   before activation.
+   */
+  async activateItem(menuItemID, data) {
+    await this.openMenu();
+    let hiddenPromise = BrowserTestUtils.waitForEvent(
+      this.menu.menupopup,
+      "popuphidden"
+    );
+    let item = document.getElementById(menuItemID);
+    if (data) {
+      this.checkItem(item, data);
+    }
+    this.menu.menupopup.activateItem(item);
+    await hiddenPromise;
+    await new Promise(resolve => setTimeout(resolve));
   }
 }
 

mail/components/customizableui/content/panelUI.inc.xhtml

@@ -488,7 +488,6 @@
                        oncommand="ToggleFavoriteFolderFlag();"/>
         <toolbarbutton id="appmenu_properties"
                        class="subviewbutton subviewbutton-iconic"
-                       label="&folderPropsCmd2.label;"
                        command="cmd_properties"/>
       </vbox>
     </panelview>

mail/components/customizableui/content/panelUI.js

@@ -530,6 +530,19 @@ const PanelUI = {
           item.disabled = !canBeCompact;
         }
       }
+
+      let propertiesMenuItem = document.getElementById("appmenu_properties");
+      if (about3Pane.gFolder?.server.type == "nntp") {
+        document.l10n.setAttributes(
+          propertiesMenuItem,
+          "menu-edit-newsgroup-properties"
+        );
+      } else {
+        document.l10n.setAttributes(
+          propertiesMenuItem,
+          "menu-edit-folder-properties"
+        );
+      }
     }
 
     InitAppFolderViewsMenu();

mail/locales/en-US/chrome/messenger/messenger.dtd

@@ -98,10 +98,6 @@
 <!ENTITY selectFlaggedCmd.accesskey "S">
 <!ENTITY menuFavoriteFolder.label "Favorite Folder">
 <!ENTITY menuFavoriteFolder.accesskey "v">
-<!ENTITY folderPropsCmd2.label "Properties">
-<!ENTITY folderPropsFolderCmd2.label "Folder Properties">
-<!ENTITY folderPropsNewsgroupCmd2.label "Newsgroup Properties">
-<!ENTITY folderPropsCmd.accesskey "o">
 <!ENTITY undoDeleteMsgCmd.label "Undo Delete Message">
 <!ENTITY redoDeleteMsgCmd.label "Redo Delete Message">
 <!ENTITY undoMoveMsgCmd.label "Undo Move Message">

mail/locales/en-US/messenger/messenger.ftl

@@ -181,6 +181,18 @@ menu-edit-undelete-messages =
         }
     .accesskey = d
 
+menu-edit-properties =
+    .label = Properties
+    .accesskey = o
+
+menu-edit-folder-properties =
+    .label = Folder Properties
+    .accesskey = o
+
+menu-edit-newsgroup-properties =
+    .label = Newsgroup Properties
+    .accesskey = o
+
 ## Message Menu
 
 redirect-msg-menuitem =

python/l10n/tb_fluent_migrations/bug_1838770_properties_menu_item.py

@@ -0,0 +1,56 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+from fluent.migratetb import COPY
+
+import fluent.syntax.ast as FTL
+
+
+def migrate(ctx):
+    """Bug 1838770 - Edit > Folder Properties doesn't work, part {index}."""
+    source = "mail/chrome/messenger/messenger.dtd"
+    dest = "mail/messenger/messenger.ftl"
+    ctx.add_transforms(
+        dest,
+        dest,
+        [
+            FTL.Message(
+                id=FTL.Identifier("menu-edit-properties"),
+                attributes=[
+                    FTL.Attribute(
+                        id=FTL.Identifier("label"), value=COPY(source, "folderPropsCmd2.label")
+                    ),
+                    FTL.Attribute(
+                        id=FTL.Identifier("accesskey"),
+                        value=COPY(source, "folderPropsCmd.accesskey"),
+                    ),
+                ],
+            ),
+            FTL.Message(
+                id=FTL.Identifier("menu-edit-folder-properties"),
+                attributes=[
+                    FTL.Attribute(
+                        id=FTL.Identifier("label"),
+                        value=COPY(source, "folderPropsFolderCmd2.label"),
+                    ),
+                    FTL.Attribute(
+                        id=FTL.Identifier("accesskey"),
+                        value=COPY(source, "folderPropsCmd.accesskey"),
+                    ),
+                ],
+            ),
+            FTL.Message(
+                id=FTL.Identifier("menu-edit-newsgroup-properties"),
+                attributes=[
+                    FTL.Attribute(
+                        id=FTL.Identifier("label"),
+                        value=COPY(source, "folderPropsNewsgroupCmd2.label"),
+                    ),
+                    FTL.Attribute(
+                        id=FTL.Identifier("accesskey"),
+                        value=COPY(source, "folderPropsCmd.accesskey"),
+                    ),
+                ],
+            ),
+        ],
+    )