tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 055d71eced482359656286f758441db36227a9de
parent c16e6f5233563ae0463355225744bd6cee615a8c
Author: Cristina Horotan <chorotan@mozilla.com>
Date:   Mon,  6 Oct 2025 13:36:50 +0300

Revert "Bug 1924535 - Remove privacy.query_stripping.strip_on_share.canDisable r=manuel,urlbar-reviewers,mak" for causing bc failures on browser_strip_on_share.js

This reverts commit 613c818f5c45ba120180399aa16fde1ffc292820.

Diffstat:
Mbrowser/base/content/nsContextMenu.sys.mjs | 26++++++++++++++++++++------
Mbrowser/base/content/test/contextMenu/browser_contextmenu.js | 27++++++++++++++-------------
Mbrowser/base/content/test/contextMenu/browser_copy_link_to_highlight.js | 14++++++++++----
Mbrowser/base/content/test/contextMenu/browser_strip_on_share_link.js | 127++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
Mbrowser/base/content/test/contextMenu/browser_strip_on_share_nested_link.js | 1+
Mbrowser/components/urlbar/UrlbarInput.sys.mjs | 27++++++++++++++++++++++-----
Mbrowser/components/urlbar/tests/browser/browser_strip_on_share.js | 141++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
Mbrowser/components/urlbar/tests/browser/browser_strip_on_share_telemetry.js | 1+
Mmodules/libpref/init/all.js | 3+++
9 files changed, 261 insertions(+), 106 deletions(-)

diff --git a/browser/base/content/nsContextMenu.sys.mjs b/browser/base/content/nsContextMenu.sys.mjs @@ -59,6 +59,13 @@ XPCOMUtils.defineLazyPreferenceGetter( XPCOMUtils.defineLazyPreferenceGetter( lazy, + "STRIP_ON_SHARE_CAN_DISABLE", + "privacy.query_stripping.strip_on_share.canDisable", + false +); + +XPCOMUtils.defineLazyPreferenceGetter( + lazy, "PDFJS_ENABLE_COMMENT", "pdfjs.enableComment", false @@ -499,13 +506,18 @@ export class nsContextMenu { // enable menu items when a text fragment can be built if (this.textFragmentURL) { - this.setItemAttr("context-copy-link-to-highlight", "disabled", null); - let link = this.getLinkURI(this.textFragmentURL); - let disabledAttr = this.#canStripParams(link) ? null : true; + this.setItemAttr("context-copy-link-to-highlight", "disabled", false); + + // only enables the clean link based on preference and canStripForShare() + // this follows the same pattern as https://bugzilla.mozilla.org/show_bug.cgi?id=1895334 + let canNotStripTextFragmentParams = + lazy.STRIP_ON_SHARE_CAN_DISABLE && + !this.#canStripParams(this.getLinkURI(this.textFragmentURL)); + this.setItemAttr( "context-copy-clean-link-to-highlight", "disabled", - disabledAttr + canNotStripTextFragmentParams ); } } @@ -1093,8 +1105,10 @@ export class nsContextMenu { !this.isSecureAboutPage() ); - let disabledAttr = this.#canStripParams() ? null : true; - this.setItemAttr("context-stripOnShareLink", "disabled", disabledAttr); + let canNotStrip = + lazy.STRIP_ON_SHARE_CAN_DISABLE && !this.#canStripParams(); + + this.setItemAttr("context-stripOnShareLink", "disabled", canNotStrip); let copyLinkSeparator = this.document.getElementById( "context-sep-copylink" diff --git a/browser/base/content/test/contextMenu/browser_contextmenu.js b/browser/base/content/test/contextMenu/browser_contextmenu.js @@ -80,6 +80,7 @@ add_setup(async function () { set: [ ["test.wait300msAfterTabSwitch", true], ["browser.search.separatePrivateDefault.ui.enabled", true], + ["privacy.query_stripping.strip_on_share.canDisable", false], ["dom.text_fragments.create_text_fragment.enabled", true], ], }); @@ -117,7 +118,7 @@ add_task(async function test_xul_text_link_label() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -214,7 +215,7 @@ const kLinkItems = [ true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -287,7 +288,7 @@ add_task(async function test_linkpreviewcommand() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -333,7 +334,7 @@ add_task(async function test_linkpreviewcommand_disabled() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -1833,7 +1834,7 @@ add_task(async function test_select_text_link() { true, "context-savelink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-copy", @@ -1906,7 +1907,7 @@ add_task(async function test_imagelink() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-viewimage", @@ -2151,7 +2152,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2188,7 +2189,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2225,7 +2226,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2264,7 +2265,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2301,7 +2302,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2338,7 +2339,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", @@ -2420,7 +2421,7 @@ add_task(async function test_background_image() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), "---", null, "context-searchselect", diff --git a/browser/base/content/test/contextMenu/browser_copy_link_to_highlight.js b/browser/base/content/test/contextMenu/browser_copy_link_to_highlight.js @@ -10,6 +10,7 @@ add_setup(async function () { set: [ ["privacy.query_stripping.strip_list", "stripParam"], ["privacy.query_stripping.strip_on_share.enabled", true], + ["privacy.query_stripping.strip_on_share.canDisable", false], ["dom.text_fragments.create_text_fragment.enabled", true], ], }); @@ -77,7 +78,8 @@ add_task(async function isVisibleIfSelection() { // tests for enabled menu items Assert.ok( - !copyLinkToHighlight.disabled, + !copyLinkToHighlight.hasAttribute("disabled") || + copyLinkToHighlight.getAttribute("disabled") === "false", "Copy Link to Highlight Menu item is enabled" ); }, @@ -93,7 +95,9 @@ add_task(async function copiesToClipboard() { "https://www.example.com/?stripParam=1234#:~:text=eiusmod%20tempor%20incididunt&text=labore", async () => { await BrowserTestUtils.waitForCondition( - () => !copyLinkToHighlight.disabled, + () => + !copyLinkToHighlight.hasAttribute("disabled") || + copyLinkToHighlight.getAttribute("disabled") === "false", "Waiting for copyLinkToHighlight to become enabled" ); copyLinkToHighlight @@ -106,7 +110,7 @@ add_task(async function copiesToClipboard() { }); // Clicking "Copy Clean Link to Highlight" copies the URL with text fragment and without tracking query params to the clipboard -add_task(async function copiesCleanLinkToClipboard() { +add_task(async function copiesToClipboard() { await testCopyLinkToHighlight({ testPage: loremIpsumTestPage(true), runTests: async ({ copyCleanLinkToHighlight }) => { @@ -114,7 +118,9 @@ add_task(async function copiesCleanLinkToClipboard() { "https://www.example.com/#:~:text=eiusmod%20tempor%20incididunt&text=labore", async () => { await BrowserTestUtils.waitForCondition( - () => !copyCleanLinkToHighlight.disabled, + () => + !copyCleanLinkToHighlight.hasAttribute("disabled") || + copyCleanLinkToHighlight.getAttribute("disabled") === "false", "Waiting for copyLinkToHighlight to become enabled" ); copyCleanLinkToHighlight diff --git a/browser/base/content/test/contextMenu/browser_strip_on_share_link.js b/browser/base/content/test/contextMenu/browser_strip_on_share_link.js @@ -13,6 +13,7 @@ add_setup(async function () { set: [ ["test.wait300msAfterTabSwitch", true], ["privacy.query_stripping.strip_list", "stripParam"], + ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); @@ -37,7 +38,8 @@ add_task(async function testPrefDisabled() { strippedURI: shortenedUrl, prefEnabled: false, useTestList: false, - expectedDisabled: true, + canDisable: false, + menuItemVisible: false, }); }); @@ -50,7 +52,8 @@ add_task(async function testQueryParamIsStrippedSelectURL() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - expectedDisabled: false, + canDisable: false, + menuItemVisible: true, }); }); @@ -63,11 +66,12 @@ add_task(async function testQueryParamIsStripped() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - expectedDisabled: false, + canDisable: false, + menuItemVisible: true, }); }); -// Menu item should be disabled if the url remains the same. +// Menu item should be visible, if there is nothing to strip, url should remain the same add_task(async function testURLIsCopiedWithNoParams() { let validUrl = "https://www.example.com/"; let shortenedUrl = "https://www.example.com/"; @@ -76,7 +80,8 @@ add_task(async function testURLIsCopiedWithNoParams() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - expectedDisabled: true, + canDisable: false, + menuItemVisible: true, }); }); @@ -89,7 +94,8 @@ add_task(async function testQueryParamIsStrippedForSiteSpecific() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: false, + canDisable: false, + menuItemVisible: true, }); }); @@ -102,7 +108,22 @@ add_task(async function testQueryParamIsNotStrippedForWrongSiteSpecific() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: true, + canDisable: false, + menuItemVisible: true, + }); +}); + +// Ensuring clean copy works with magnet links. We don't strip anything but copying the original URI should still work. +add_task(async function testMagneticLinks() { + let validUrl = "magnet:?xt=urn:btih:somesha1hash"; + let shortenedUrl = "magnet:?xt=urn:btih:somesha1hash"; + await testStripOnShare({ + originalURI: validUrl, + strippedURI: shortenedUrl, + prefEnabled: true, + useTestList: true, + canDisable: false, + menuItemVisible: true, }); }); @@ -115,12 +136,13 @@ add_task(async function testMagneticLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: true, + canDisable: true, + menuItemVisible: false, }); }); // Ensuring clean copy is disabled on about links -add_task(async function testAboutLinks() { +add_task(async function testMagneticLinks() { let validUrl = "about:blank"; let shortenedUrl = "about:blank"; await testStripOnShare({ @@ -128,20 +150,22 @@ add_task(async function testAboutLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: true, + canDisable: true, + menuItemVisible: false, }); }); -// Ensure clean copy is disabled when nothing can be stripped. +// Ensure clean copy is diabled when nothing can be stripped add_task(async function testStripNothingDisabled() { let validUrl = "https://example.com"; - let shortenedUrl = "https://example.com/"; + let shortenedUrl = "https://example.com"; await testStripOnShare({ originalURI: validUrl, strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: true, + canDisable: true, + menuItemVisible: false, }); }); @@ -157,32 +181,65 @@ add_task(async function testErrorHandlingForNestedLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - expectedDisabled: false, + canDisable: false, + menuItemVisible: true, + }); +}); + +// Tests that the menu item does not show up if the strip on share and disable +// pref is enabled and there is nothing to strip +add_task(async function testNoParamToStripWithCanDisablePref() { + let validUrl = "https://www.example.com/"; + let shortenedUrl = "https://www.example.com/"; + await testStripOnShare({ + originalURI: validUrl, + strippedURI: shortenedUrl, + prefEnabled: true, + useTestList: false, + canDisable: true, + menuItemVisible: false, + }); +}); + +// Menu item should be visible, url should be stripped. +// Even with disable pref enabed +add_task(async function testQueryParamIsStrippedWithCanDisablePref() { + let validUrl = "https://www.example.com/?stripParam=1234"; + let shortenedUrl = "https://www.example.com/"; + await testStripOnShare({ + originalURI: validUrl, + strippedURI: shortenedUrl, + prefEnabled: true, + useTestList: false, + canDisable: true, + menuItemVisible: true, }); }); /** - * Opens a new tab, opens the context menu and checks the menu item. + * Opens a new tab, opens the context menu and checks that the strip-on-share menu item is visible. * Checks that the stripped version of the url is copied to the clipboard. * - * @param {object} options - * @param {string} options.originalURI - The orginal url before stripping. - * @param {string} options.strippedURI - The expected url after stripping. - * @param {boolean} options.prefEnabled - If true, enable strip_on_share pref. - * @param {boolean} options.useTestList - If true, use test mode pref and list. - * @param {boolean} options.expectedDisabled - The expected item disabled state. + * @param {string} originalURI - The orginal url before the stripping occurs + * @param {string} strippedURI - The expected url after stripping occurs + * @param {boolean} prefEnabled - Whether StripOnShare pref is enabled + * @param {boolean} useTestList - Whether the StripOnShare or Test list should be used + * @param {boolean} canDisable - Whether the StripOnShare context menu item is disabled if there is nothing to be stripped + * @param {boolean} menuItemVisible - Whether the StripOnShare context menu item should be visible */ async function testStripOnShare({ originalURI, strippedURI, prefEnabled, useTestList, - expectedDisabled, + canDisable, + menuItemVisible, }) { await SpecialPowers.pushPrefEnv({ set: [ ["privacy.query_stripping.strip_on_share.enabled", prefEnabled], ["privacy.query_stripping.strip_on_share.enableTestMode", useTestList], + ["privacy.query_stripping.strip_on_share.canDisable", canDisable], ], }); @@ -230,25 +287,29 @@ async function testStripOnShare({ browser ); await awaitPopupShown; - - let menuItem = contextMenu.querySelector("#context-stripOnShareLink"); - Assert.equal( - BrowserTestUtils.isVisible(menuItem), - prefEnabled, - "Menu item is visible" - ); - Assert.equal(menuItem.disabled, expectedDisabled, "Menu item is disabled"); - let awaitPopupHidden = BrowserTestUtils.waitForEvent( contextMenu, "popuphidden" ); - if (prefEnabled) { + let stripOnShare = contextMenu.querySelector("#context-stripOnShareLink"); + if (menuItemVisible) { + Assert.ok( + BrowserTestUtils.isVisible(stripOnShare), + "Menu item is visible" + ); // Make sure the stripped link will be copied to the clipboard await SimpleTest.promiseClipboardChange(strippedURI, () => { - contextMenu.activateItem(menuItem); + contextMenu.activateItem(stripOnShare); }); } else { + if (canDisable) { + Assert.ok(stripOnShare.disabled, "Menu item is disabled"); + } else { + Assert.ok( + !BrowserTestUtils.isVisible(stripOnShare), + "Menu item is not visible" + ); + } contextMenu.hidePopup(); } await awaitPopupHidden; diff --git a/browser/base/content/test/contextMenu/browser_strip_on_share_nested_link.js b/browser/base/content/test/contextMenu/browser_strip_on_share_nested_link.js @@ -13,6 +13,7 @@ add_setup(async function () { set: [ ["test.wait300msAfterTabSwitch", true], ["privacy.query_stripping.strip_list", "stripParam"], + ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs @@ -73,6 +73,13 @@ XPCOMUtils.defineLazyPreferenceGetter( false ); +XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "STRIP_ON_SHARE_CAN_DISABLE", + "privacy.query_stripping.strip_on_share.canDisable", + false +); + ChromeUtils.defineLazyGetter(lazy, "logger", () => lazy.UrlbarUtils.getLogger({ prefix: "Input" }) ); @@ -3871,15 +3878,25 @@ export class UrlbarInput { } let controller = this.document.commandDispatcher.getControllerForCommand("cmd_copy"); - if ( - !controller.isCommandEnabled("cmd_copy") || - !this.#isClipboardURIValid() - ) { + // url bar is empty + if (!controller.isCommandEnabled("cmd_copy")) { stripOnShare.setAttribute("hidden", true); return; } + // selection is not a valid url + if (!this.#isClipboardURIValid()) { + stripOnShare.setAttribute("hidden", true); + return; + } + if (lazy.STRIP_ON_SHARE_CAN_DISABLE) { + if (!this.#canStrip()) { + stripOnShare.removeAttribute("hidden"); + stripOnShare.setAttribute("disabled", true); + return; + } + } stripOnShare.removeAttribute("hidden"); - stripOnShare.toggleAttribute("disabled", !this.#canStrip()); + stripOnShare.removeAttribute("disabled"); }); } diff --git a/browser/components/urlbar/tests/browser/browser_strip_on_share.js b/browser/components/urlbar/tests/browser/browser_strip_on_share.js @@ -12,6 +12,7 @@ add_setup(async function () { set: [ ["privacy.query_stripping.strip_list", "stripParam"], ["privacy.query_stripping.enabled", false], + ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); @@ -25,20 +26,22 @@ add_setup(async function () { // Selection is not a valid URI, menu item should be hidden add_task(async function testInvalidURI() { - await testMenuItemDisabled({ - url: "https://www.example.com/?stripParam=1234", - prefEnabled: true, - selection: true, - }); + await testMenuItemDisabled( + "https://www.example.com/?stripParam=1234", + true, + false, + true + ); }); // Pref is not enabled, menu item should be hidden add_task(async function testPrefDisabled() { - await testMenuItemDisabled({ - url: "https://www.example.com/?stripParam=1234", - prefEnabled: false, - selection: false, - }); + await testMenuItemDisabled( + "https://www.example.com/?stripParam=1234", + false, + false, + false + ); }); // Menu item should be visible, the whole url is copied without a selection, url should be stripped. @@ -50,7 +53,8 @@ add_task(async function testQueryParamIsStripped() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - expectedDisabled: false, + canDisable: false, + isDisabled: false, }); }); @@ -63,12 +67,28 @@ add_task(async function testQueryParamIsStrippedSelectURL() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - expectedDisabled: false, + canDisable: false, + isDisabled: false, + }); +}); + +// Menu item should be visible even with the disable pref enabled +// selecting the whole url, url should be stripped. +add_task(async function testQueryParamIsStrippedWithDisabletPref() { + let originalUrl = "https://www.example.com/?stripParam=1234"; + let shortenedUrl = "https://www.example.com/"; + await testMenuItemEnabled({ + selectWholeUrl: true, + validUrl: originalUrl, + strippedUrl: shortenedUrl, + useTestList: false, + canDisable: true, + isDisabled: false, }); }); // Make sure other parameters don't interfere with stripping -add_task(async function testQueryParamIsStrippedWithOtherParam() { +add_task(async function testQueryParamIsStrippedWithDisabletPref() { let originalUrl = "https://www.example.com/?keepParameter=1&stripParam=1234"; let shortenedUrl = "https://www.example.com/?keepParameter=1"; await testMenuItemEnabled({ @@ -76,18 +96,20 @@ add_task(async function testQueryParamIsStrippedWithOtherParam() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - expectedDisabled: false, + canDisable: true, + isDisabled: false, }); }); // Test that menu item becomes visible again after selecting a non-url -add_task(async function testQueryParamIsStrippedAfterInvalid() { +add_task(async function testQueryParamIsStrippedWithDisabletPref() { // Selection is not a valid URI, menu item should be hidden - await testMenuItemDisabled({ - url: "https://www.example.com/?stripParam=1234", - prefEnabled: true, - selection: true, - }); + await testMenuItemDisabled( + "https://www.example.com/?stripParam=1234", + true, + true, + true + ); // test if menu item is visible after it getting hidden let originalUrl = "https://www.example.com/?stripParam=1234"; let shortenedUrl = "https://www.example.com/"; @@ -96,11 +118,12 @@ add_task(async function testQueryParamIsStrippedAfterInvalid() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - expectedDisabled: false, + canDisable: true, + isDisabled: false, }); }); -// Menu item should be disabled if url is the same. +// Menu item should be visible, selecting the whole url, url should be the same. add_task(async function testURLIsCopiedWithNoParams() { let originalUrl = "https://www.example.com/"; let shortenedUrl = "https://www.example.com/"; @@ -109,7 +132,22 @@ add_task(async function testURLIsCopiedWithNoParams() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - expectedDisabled: true, + canDisable: false, + isDisabled: false, + }); +}); + +// Menu item should be visible, selecting the whole url, url should be the same. +add_task(async function testURLIsCopiedWithNoParams() { + let originalUrl = "https://www.example.com/"; + let shortenedUrl = "https://www.example.com/"; + await testMenuItemEnabled({ + selectWholeUrl: true, + validUrl: originalUrl, + strippedUrl: shortenedUrl, + useTestList: false, + canDisable: true, + isDisabled: true, }); }); @@ -122,7 +160,8 @@ add_task(async function testQueryParamIsStrippedForSiteSpecific() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - expectedDisabled: false, + canDisable: false, + isDisabled: false, }); }); @@ -135,7 +174,8 @@ add_task(async function testQueryParamIsNotStrippedForWrongSiteSpecific() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - expectedDisabled: true, + canDisable: false, + isDisabled: false, }); }); @@ -149,13 +189,14 @@ add_task(async function testQueryParamIsStrippedWhenParamIsCapitalized() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - expectedDisabled: false, + canDisable: false, + isDisabled: false, }); }); // Ensuring site specific parameters are stripped regardless // of capitalization in the URI -add_task(async function testQueryParamIsStrippedWhenParamIsLowercase() { +add_task(async function testQueryParamIsStrippedWhenParamIsCapitalized() { let originalUrl = "https://www.example.com/?test_5=1234"; let shortenedUrl = "https://www.example.com/"; await testMenuItemEnabled({ @@ -163,21 +204,25 @@ add_task(async function testQueryParamIsStrippedWhenParamIsLowercase() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - expectedDisabled: false, + canDisable: false, + isDisabled: false, }); }); /** - * Opens a new tab and checks menu item is hidden in the url bar context menu. + * Opens a new tab, opens the url bar context menu and checks that the strip-on-share menu item is not visible * - * @param {object} options - * @param {string} options.url - The url to be loaded. - * @param {boolean} options.prefEnabled - If true, enable strip_on_share pref. - * @param {boolean} options.selection - If true, select only part of the url. + * @param {string} url - The url to be loaded + * @param {boolean} prefEnabled - Whether privacy.query_stripping.strip_on_share.enabled should be enabled for the test + * @param {boolean} canDisable - Whether privacy.query_stripping.strip_on_share.canDisable should be enabled for the test + * @param {boolean} selection - True: The whole url will be selected, false: Only part of the url will be selected */ -async function testMenuItemDisabled({ url, prefEnabled, selection }) { +async function testMenuItemDisabled(url, prefEnabled, canDisable, selection) { await SpecialPowers.pushPrefEnv({ - set: [["privacy.query_stripping.strip_on_share.enabled", prefEnabled]], + set: [ + ["privacy.query_stripping.strip_on_share.enabled", prefEnabled], + ["privacy.query_stripping.strip_on_share.canDisable", canDisable], + ], }); await BrowserTestUtils.withNewTab(url, async function () { @@ -202,27 +247,30 @@ async function testMenuItemDisabled({ url, prefEnabled, selection }) { } /** - * Opens a new tab and checks menu item is visible in the url bar context menu. + * Opens a new tab, opens the url bar context menu and checks that the strip-on-share menu item is visible. * Checks that the stripped version of the url is copied to the clipboard. * - * @param {object} options - * @param {boolean} options.selectWholeUrl - If true, select the whole url. - * @param {string} options.validUrl - The original url before stripping. - * @param {string} options.strippedUrl - The expected url after stripping. - * @param {boolean} options.useTestList - If true, use test mode pref and list. - * @param {boolean} options.expectedDisabled - The expected iten disabled state. + * @param {object} options - method options + * @param {boolean} options.selectWholeUrl - Whether the whole url should be selected + * @param {string} options.validUrl - The original url before the stripping occurs + * @param {string} options.strippedUrl - The expected url after stripping occurs + * @param {boolean} options.useTestList - Whether the StripOnShare or Test list should be used + * @param {boolean} options.canDisable - Whether privacy.query_stripping.strip_on_share.canDisable should be enabled for the test + * @param {boolean} options.isDisabled - Whether the menu item is visible, but disabled */ async function testMenuItemEnabled({ selectWholeUrl, validUrl, strippedUrl, useTestList, - expectedDisabled, + canDisable, + isDisabled, }) { await SpecialPowers.pushPrefEnv({ set: [ ["privacy.query_stripping.strip_on_share.enabled", true], ["privacy.query_stripping.strip_on_share.enableTestMode", useTestList], + ["privacy.query_stripping.strip_on_share.canDisable", canDisable], ], }); @@ -250,10 +298,13 @@ async function testMenuItemEnabled({ if (selectWholeUrl) { gURLBar.select(); } - let menuitem = await promiseContextualMenuitem("strip-on-share"); Assert.ok(BrowserTestUtils.isVisible(menuitem), "Menu item is visible"); - Assert.equal(menuitem.disabled, expectedDisabled, "Menu item is disabled"); + if (isDisabled) { + Assert.ok(menuitem.getAttribute("disabled"), "Menu item is greyed out"); + } else { + Assert.ok(!menuitem.getAttribute("disabled"), "Menu item is interactive"); + } let hidePromise = BrowserTestUtils.waitForEvent( menuitem.parentElement, diff --git a/browser/components/urlbar/tests/browser/browser_strip_on_share_telemetry.js b/browser/components/urlbar/tests/browser/browser_strip_on_share_telemetry.js @@ -14,6 +14,7 @@ add_setup(async function () { set: [ ["privacy.query_stripping.strip_on_share.enabled", true], ["privacy.query_stripping.enabled", false], + ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js @@ -4117,6 +4117,9 @@ pref("privacy.query_stripping.strip_on_share.enableTestMode", false); pref("toolkit.backgroundtasks.tests.geckoPrefsOverriden", 18); #endif +// To disable the Strip on Share context menu option if nothing can be stripped +pref("privacy.query_stripping.strip_on_share.canDisable", true); + // Captcha Detection pref("captchadetection.loglevel", "Warn"); pref("captchadetection.actor.enabled", true);