[tbb-commits] [Git][tpo/applications/mullvad-browser][mullvad-browser-128.2.0esr-14.0-1] 2 commits: Bug 1919363 - Only show one app menu "new window" item in permanent private browsing. r=mconley

morgan (@morgan) git at gitlab.torproject.org
Mon Sep 23 18:21:29 UTC 2024



morgan pushed to branch mullvad-browser-128.2.0esr-14.0-1 at The Tor Project / Applications / Mullvad Browser


Commits:
db69ea11 by Henry Wilkes at 2024-09-23T18:20:35+00:00
Bug 1919363 - Only show one app menu "new window" item in permanent private browsing. r=mconley

We also update the browser_private_browsing_window.js test.
The previous test was limited because it was referring to non-existent
"appmenu_newNavigator" and "appmenu_newPrivateWindow".

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

- - - - -
3ff1d289 by Henry Wilkes at 2024-09-23T18:20:35+00:00
fixup! Bug 18905: Hide unwanted items from help menu

Bug 42362: Stop hiding the Tools:PrivateBrowsing command and shortcut.

- - - - -


3 changed files:

- browser/base/content/browser-init.js
- browser/base/content/browser.js
- browser/base/content/test/general/browser_private_browsing_window.js


Changes:

=====================================
browser/base/content/browser-init.js
=====================================
@@ -289,10 +289,7 @@ var gBrowserInit = {
     this._boundDelayedStartup = this._delayedStartup.bind(this);
     window.addEventListener("MozAfterPaint", this._boundDelayedStartup);
 
-    if (
-      PrivateBrowsingUtils.permanentPrivateBrowsing ||
-      !PrivateBrowsingUtils.enabled
-    ) {
+    if (!PrivateBrowsingUtils.enabled) {
       document.getElementById("Tools:PrivateBrowsing").hidden = true;
       // Setting disabled doesn't disable the shortcut, so we just remove
       // the keybinding.


=====================================
browser/base/content/browser.js
=====================================
@@ -6797,15 +6797,34 @@ var gPrivateBrowsingUI = {
     }
 
     if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
-      // Adjust the New Window menu entries
-      let newWindow = document.getElementById("menu_newNavigator");
-      let newPrivateWindow = document.getElementById("menu_newPrivateWindow");
-      if (newWindow && newPrivateWindow) {
-        newPrivateWindow.hidden = true;
-        newWindow.label = newPrivateWindow.label;
-        newWindow.accessKey = newPrivateWindow.accessKey;
-        newWindow.command = newPrivateWindow.command;
-      }
+      let hideNewWindowItem = (windowItem, privateWindowItem) => {
+        // In permanent browsing mode command "cmd_newNavigator" should act the
+        // same as "Tools:PrivateBrowsing".
+        // So we hide the redundant private window item. But we also rename the
+        // "new window" item to be "new private window".
+        // NOTE: We choose to hide privateWindowItem rather than windowItem so
+        // that we still show the "key" for "cmd_newNavigator" (Ctrl+N) rather
+        // than (Ctrl+Shift+P).
+        privateWindowItem.hidden = true;
+        windowItem.setAttribute(
+          "data-l10n-id",
+          privateWindowItem.getAttribute("data-l10n-id")
+        );
+      };
+
+      // Adjust the File menu items.
+      hideNewWindowItem(
+        document.getElementById("menu_newNavigator"),
+        document.getElementById("menu_newPrivateWindow")
+      );
+      // Adjust the App menu items.
+      hideNewWindowItem(
+        PanelMultiView.getViewNode(document, "appMenu-new-window-button2"),
+        PanelMultiView.getViewNode(
+          document,
+          "appMenu-new-private-window-button2"
+        )
+      );
     }
   },
 };


=====================================
browser/base/content/test/general/browser_private_browsing_window.js
=====================================
@@ -1,133 +1,218 @@
-// Make sure that we can open private browsing windows
+/* 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/. */
 
-function test() {
-  waitForExplicitFinish();
-  var nonPrivateWin = OpenBrowserWindow();
-  ok(
-    !PrivateBrowsingUtils.isWindowPrivate(nonPrivateWin),
+add_task(async function testOpenBrowserWindow() {
+  let win = OpenBrowserWindow();
+  Assert.ok(
+    !PrivateBrowsingUtils.isWindowPrivate(win),
     "OpenBrowserWindow() should open a normal window"
   );
-  nonPrivateWin.close();
+  await BrowserTestUtils.closeWindow(win);
 
-  var privateWin = OpenBrowserWindow({ private: true });
-  ok(
-    PrivateBrowsingUtils.isWindowPrivate(privateWin),
+  win = OpenBrowserWindow({ private: true });
+  Assert.ok(
+    PrivateBrowsingUtils.isWindowPrivate(win),
     "OpenBrowserWindow({private: true}) should open a private window"
   );
+  await BrowserTestUtils.closeWindow(win);
 
-  nonPrivateWin = OpenBrowserWindow({ private: false });
-  ok(
-    !PrivateBrowsingUtils.isWindowPrivate(nonPrivateWin),
+  win = OpenBrowserWindow({ private: false });
+  Assert.ok(
+    !PrivateBrowsingUtils.isWindowPrivate(win),
     "OpenBrowserWindow({private: false}) should open a normal window"
   );
-  nonPrivateWin.close();
+  await BrowserTestUtils.closeWindow(win);
 
-  whenDelayedStartupFinished(privateWin, function () {
-    nonPrivateWin = privateWin.OpenBrowserWindow({ private: false });
-    ok(
-      !PrivateBrowsingUtils.isWindowPrivate(nonPrivateWin),
-      "privateWin.OpenBrowserWindow({private: false}) should open a normal window"
-    );
+  // In permanent browsing mode.
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.privatebrowsing.autostart", true]],
+  });
+
+  win = OpenBrowserWindow();
+  Assert.ok(
+    PrivateBrowsingUtils.isWindowPrivate(win),
+    "OpenBrowserWindow() in PBM should open a private window"
+  );
+  await BrowserTestUtils.closeWindow(win);
+
+  win = OpenBrowserWindow({ private: true });
+  Assert.ok(
+    PrivateBrowsingUtils.isWindowPrivate(win),
+    "OpenBrowserWindow({private: true}) in PBM should open a private window"
+  );
+  await BrowserTestUtils.closeWindow(win);
+
+  win = OpenBrowserWindow({ private: false });
+  Assert.ok(
+    PrivateBrowsingUtils.isWindowPrivate(win),
+    "OpenBrowserWindow({private: false}) in PBM should open a private window"
+  );
+  await BrowserTestUtils.closeWindow(win);
+
+  await SpecialPowers.popPrefEnv();
+});
 
-    nonPrivateWin.close();
-
-    [
-      {
-        normal: "menu_newNavigator",
-        private: "menu_newPrivateWindow",
-        accesskey: true,
-      },
-      {
-        normal: "appmenu_newNavigator",
-        private: "appmenu_newPrivateWindow",
-        accesskey: false,
-      },
-    ].forEach(function (menu) {
-      let newWindow = privateWin.document.getElementById(menu.normal);
-      let newPrivateWindow = privateWin.document.getElementById(menu.private);
-      if (newWindow && newPrivateWindow) {
-        ok(
-          !newPrivateWindow.hidden,
-          "New Private Window menu item should be hidden"
-        );
-        isnot(
-          newWindow.label,
-          newPrivateWindow.label,
-          "New Window's label shouldn't be overwritten"
-        );
-        if (menu.accesskey) {
-          isnot(
-            newWindow.accessKey,
-            newPrivateWindow.accessKey,
-            "New Window's accessKey shouldn't be overwritten"
-          );
-        }
-        isnot(
-          newWindow.command,
-          newPrivateWindow.command,
-          "New Window's command shouldn't be overwritten"
-        );
-      }
-    });
-
-    is(
-      privateWin.gBrowser.tabs[0].label,
-      "New Private Tab",
-      "New tabs in the private browsing windows should have 'New Private Tab' as the title."
+/**
+ * Check that the "new window" menu items have the expected properties.
+ *
+ * @param {Element} newWindowItem - The "new window" item to check.
+ * @param {Element} privateWindowItem - The "new private window" item to check.
+ * @param {Object} expect - The expected properties.
+ * @param {boolean} expect.privateVisible - Whether we expect the private item
+ *   to be visible or not.
+ * @param {string} expect.newWindowL10nId - The expected string ID used by the
+ *   "new window" item.
+ * @param {string} expect.privateWindowL10nId - The expected string ID used by
+ *   the "new private window" item.
+ * @param {boolean} [useIsVisible=true] - Whether to test the "true" visibility
+ *   of the item. Otherwise only the "hidden" attribute is checked.
+ */
+function assertMenuItems(
+  newWindowItem,
+  privateWindowItem,
+  expect,
+  useIsVisible = true
+) {
+  Assert.ok(newWindowItem);
+  Assert.ok(privateWindowItem);
+
+  if (useIsVisible) {
+    Assert.ok(
+      BrowserTestUtils.isVisible(newWindowItem),
+      "New window item should be visible"
     );
+  } else {
+    // The application menu is not accessible on macOS, just check the hidden
+    // attribute.
+    Assert.ok(!newWindowItem.hidden, "New window item should be visible");
+  }
+
+  Assert.equal(
+    newWindowItem.getAttribute("key"),
+    "key_newNavigator",
+    "New window item should use the same key"
+  );
+  Assert.equal(
+    newWindowItem.getAttribute("data-l10n-id"),
+    expect.newWindowL10nId
+  );
 
-    privateWin.close();
-
-    Services.prefs.setBoolPref("browser.privatebrowsing.autostart", true);
-    privateWin = OpenBrowserWindow({ private: true });
-    whenDelayedStartupFinished(privateWin, function () {
-      [
-        {
-          normal: "menu_newNavigator",
-          private: "menu_newPrivateWindow",
-          accessKey: true,
-        },
-        {
-          normal: "appmenu_newNavigator",
-          private: "appmenu_newPrivateWindow",
-          accessKey: false,
-        },
-      ].forEach(function (menu) {
-        let newWindow = privateWin.document.getElementById(menu.normal);
-        let newPrivateWindow = privateWin.document.getElementById(menu.private);
-        if (newWindow && newPrivateWindow) {
-          ok(
-            newPrivateWindow.hidden,
-            "New Private Window menu item should be hidden"
-          );
-          is(
-            newWindow.label,
-            newPrivateWindow.label,
-            "New Window's label should be overwritten"
-          );
-          if (menu.accesskey) {
-            is(
-              newWindow.accessKey,
-              newPrivateWindow.accessKey,
-              "New Window's accessKey should be overwritten"
-            );
-          }
-          is(
-            newWindow.command,
-            newPrivateWindow.command,
-            "New Window's command should be overwritten"
-          );
-        }
-      });
-
-      is(
-        privateWin.gBrowser.tabs[0].label,
-        "New Tab",
-        "Normal tab title is used also in the permanent private browsing mode."
+  if (!expect.privateVisible) {
+    if (useIsVisible) {
+      Assert.ok(
+        BrowserTestUtils.isHidden(privateWindowItem),
+        "Private window item should be hidden"
       );
-      privateWin.close();
-      Services.prefs.clearUserPref("browser.privatebrowsing.autostart");
-      finish();
-    });
-  });
+    } else {
+      Assert.ok(
+        privateWindowItem.hidden,
+        "Private window item should be hidden"
+      );
+    }
+    // Don't check attributes since hidden.
+  } else {
+    if (useIsVisible) {
+      Assert.ok(
+        BrowserTestUtils.isVisible(privateWindowItem),
+        "Private window item should be visible"
+      );
+    } else {
+      Assert.ok(
+        !privateWindowItem.hidden,
+        "Private window item should be visible"
+      );
+    }
+    Assert.equal(
+      privateWindowItem.getAttribute("key"),
+      "key_privatebrowsing",
+      "Private window item should use the same key"
+    );
+    Assert.equal(
+      privateWindowItem.getAttribute("data-l10n-id"),
+      expect.privateWindowL10nId
+    );
+  }
+}
+
+/**
+ * Check that a window has the expected "new window" items in the "File" and app
+ * menus.
+ *
+ * @param {Window} win - The window to check.
+ * @param {boolean} expectBoth - Whether we expect the window to contain both
+ *   "new window" and "new private window" as separate controls.
+ */
+async function checkWindowMenus(win, expectBoth) {
+  // Check the File menu.
+  assertMenuItems(
+    win.document.getElementById("menu_newNavigator"),
+    win.document.getElementById("menu_newPrivateWindow"),
+    {
+      privateVisible: expectBoth,
+      // If in permanent private browsing, expect the new window item to use the
+      // "New private window" string.
+      newWindowL10nId: expectBoth
+        ? "menu-file-new-window"
+        : "menu-file-new-private-window",
+      privateWindowL10nId: "menu-file-new-private-window",
+    },
+    // The file menu is difficult to open cross-platform, so we do not open it
+    // for this test.
+    false
+  );
+
+  // Open the app menu.
+  let appMenuButton = win.document.getElementById("PanelUI-menu-button");
+  let appMenu = win.document.getElementById("appMenu-popup");
+  let menuShown = BrowserTestUtils.waitForEvent(appMenu, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(appMenuButton, {}, win);
+  await menuShown;
+
+  // Check the app menu.
+  assertMenuItems(
+    win.document.getElementById("appMenu-new-window-button2"),
+    win.document.getElementById("appMenu-new-private-window-button2"),
+    {
+      privateVisible: expectBoth,
+      // If in permanent private browsing, expect the new window item to use the
+      // "New private window" string.
+      newWindowL10nId: expectBoth
+        ? "appmenuitem-new-window"
+        : "appmenuitem-new-private-window",
+      privateWindowL10nId: "appmenuitem-new-private-window",
+    }
+  );
+
+  appMenu.hidePopup();
 }
+
+add_task(async function testNewWindowMenuItems() {
+  // In non-private window, expect both menu items.
+  let win = await BrowserTestUtils.openNewBrowserWindow({
+    private: false,
+  });
+  await checkWindowMenus(win, true);
+  Assert.equal(win.gBrowser.currentURI.spec, "about:blank");
+  await BrowserTestUtils.closeWindow(win);
+
+  // In non-permanent private window, still expect both menu items.
+  win = await BrowserTestUtils.openNewBrowserWindow({
+    private: true,
+  });
+  await checkWindowMenus(win, true);
+  Assert.equal(win.gBrowser.currentURI.spec, "about:privatebrowsing");
+  await BrowserTestUtils.closeWindow(win);
+
+  // In permanent private browsing, expect only one menu item.
+  await SpecialPowers.pushPrefEnv({
+    set: [["browser.privatebrowsing.autostart", true]],
+  });
+
+  win = await BrowserTestUtils.openNewBrowserWindow();
+  await checkWindowMenus(win, false);
+  Assert.equal(win.gBrowser.currentURI.spec, "about:blank");
+  await BrowserTestUtils.closeWindow(win);
+
+  await SpecialPowers.popPrefEnv();
+});



View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/compare/ea0d3618dea67c44390edb69f6e83452d43acc9d...3ff1d28905a3629200d35cf3bc4d2b776fd058c6

-- 
View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/compare/ea0d3618dea67c44390edb69f6e83452d43acc9d...3ff1d28905a3629200d35cf3bc4d2b776fd058c6
You're receiving this email because of your account on gitlab.torproject.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tbb-commits/attachments/20240923/4d7718dc/attachment-0001.htm>


More information about the tbb-commits mailing list