[tbb-commits] [Git][tpo/applications/tor-browser][base-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:20:22 UTC 2024
morgan pushed to branch base-browser-128.2.0esr-14.0-1 at The Tor Project / Applications / Tor Browser
Commits:
104d7d03 by Henry Wilkes at 2024-09-23T18:19:58+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
- - - - -
26180332 by Henry Wilkes at 2024-09-23T18:19:58+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
=====================================
@@ -6795,15 +6795,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/tor-browser/-/compare/60479f0b7b84e24e82fa2575aa0242ca1a25da0d...26180332375f16d957383c69b69200a6604d0c28
--
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/60479f0b7b84e24e82fa2575aa0242ca1a25da0d...26180332375f16d957383c69b69200a6604d0c28
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/ead56c79/attachment-0001.htm>
More information about the tbb-commits
mailing list