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

Commits: eebb38fa by Henry Wilkes at 2024-09-23T10:53:10+01: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

- - - - - 01972d1c by Henry Wilkes at 2024-09-23T10:56:02+01: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 ===================================== @@ -297,10 +297,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 ===================================== @@ -6839,15 +6839,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/3311152...