tor-browser

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

commit 613c818f5c45ba120180399aa16fde1ffc292820
parent 53aba3edf211844074e5b3b22fa85366cc9d2905
Author: Mathew Hodson <mathew.hodson@gmail.com>
Date:   Mon,  6 Oct 2025 09:11:51 +0000

Bug 1924535 - Remove privacy.query_stripping.strip_on_share.canDisable r=manuel,urlbar-reviewers,mak

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

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, 106 insertions(+), 261 deletions(-)

diff --git a/browser/base/content/nsContextMenu.sys.mjs b/browser/base/content/nsContextMenu.sys.mjs @@ -59,13 +59,6 @@ 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 @@ -506,18 +499,13 @@ export class nsContextMenu { // enable menu items when a text fragment can be built if (this.textFragmentURL) { - 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-link-to-highlight", "disabled", null); + let link = this.getLinkURI(this.textFragmentURL); + let disabledAttr = this.#canStripParams(link) ? null : true; this.setItemAttr( "context-copy-clean-link-to-highlight", "disabled", - canNotStripTextFragmentParams + disabledAttr ); } } @@ -1105,10 +1093,8 @@ export class nsContextMenu { !this.isSecureAboutPage() ); - let canNotStrip = - lazy.STRIP_ON_SHARE_CAN_DISABLE && !this.#canStripParams(); - - this.setItemAttr("context-stripOnShareLink", "disabled", canNotStrip); + let disabledAttr = this.#canStripParams() ? null : true; + this.setItemAttr("context-stripOnShareLink", "disabled", disabledAttr); 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,7 +80,6 @@ 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], ], }); @@ -118,7 +117,7 @@ add_task(async function test_xul_text_link_label() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -215,7 +214,7 @@ const kLinkItems = [ true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -288,7 +287,7 @@ add_task(async function test_linkpreviewcommand() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -334,7 +333,7 @@ add_task(async function test_linkpreviewcommand_disabled() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -1834,7 +1833,7 @@ add_task(async function test_select_text_link() { true, "context-savelink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-copy", @@ -1907,7 +1906,7 @@ add_task(async function test_imagelink() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-viewimage", @@ -2152,7 +2151,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2189,7 +2188,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2226,7 +2225,7 @@ add_task(async function test_svg_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2265,7 +2264,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2302,7 +2301,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2339,7 +2338,7 @@ add_task(async function test_svg_relative_link() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", null, "context-searchselect", @@ -2421,7 +2420,7 @@ add_task(async function test_background_image() { true, "context-copylink", true, - ...(hasStripOnShare ? ["context-stripOnShareLink", true] : []), + ...(hasStripOnShare ? ["context-stripOnShareLink", false] : []), "---", 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,7 +10,6 @@ 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], ], }); @@ -78,8 +77,7 @@ add_task(async function isVisibleIfSelection() { // tests for enabled menu items Assert.ok( - !copyLinkToHighlight.hasAttribute("disabled") || - copyLinkToHighlight.getAttribute("disabled") === "false", + !copyLinkToHighlight.disabled, "Copy Link to Highlight Menu item is enabled" ); }, @@ -95,9 +93,7 @@ add_task(async function copiesToClipboard() { "https://www.example.com/?stripParam=1234#:~:text=eiusmod%20tempor%20incididunt&text=labore", async () => { await BrowserTestUtils.waitForCondition( - () => - !copyLinkToHighlight.hasAttribute("disabled") || - copyLinkToHighlight.getAttribute("disabled") === "false", + () => !copyLinkToHighlight.disabled, "Waiting for copyLinkToHighlight to become enabled" ); copyLinkToHighlight @@ -110,7 +106,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 copiesToClipboard() { +add_task(async function copiesCleanLinkToClipboard() { await testCopyLinkToHighlight({ testPage: loremIpsumTestPage(true), runTests: async ({ copyCleanLinkToHighlight }) => { @@ -118,9 +114,7 @@ add_task(async function copiesToClipboard() { "https://www.example.com/#:~:text=eiusmod%20tempor%20incididunt&text=labore", async () => { await BrowserTestUtils.waitForCondition( - () => - !copyCleanLinkToHighlight.hasAttribute("disabled") || - copyCleanLinkToHighlight.getAttribute("disabled") === "false", + () => !copyCleanLinkToHighlight.disabled, "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,7 +13,6 @@ add_setup(async function () { set: [ ["test.wait300msAfterTabSwitch", true], ["privacy.query_stripping.strip_list", "stripParam"], - ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); @@ -38,8 +37,7 @@ add_task(async function testPrefDisabled() { strippedURI: shortenedUrl, prefEnabled: false, useTestList: false, - canDisable: false, - menuItemVisible: false, + expectedDisabled: true, }); }); @@ -52,8 +50,7 @@ add_task(async function testQueryParamIsStrippedSelectURL() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - canDisable: false, - menuItemVisible: true, + expectedDisabled: false, }); }); @@ -66,12 +63,11 @@ add_task(async function testQueryParamIsStripped() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - canDisable: false, - menuItemVisible: true, + expectedDisabled: false, }); }); -// Menu item should be visible, if there is nothing to strip, url should remain the same +// Menu item should be disabled if the url remains the same. add_task(async function testURLIsCopiedWithNoParams() { let validUrl = "https://www.example.com/"; let shortenedUrl = "https://www.example.com/"; @@ -80,8 +76,7 @@ add_task(async function testURLIsCopiedWithNoParams() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: false, - canDisable: false, - menuItemVisible: true, + expectedDisabled: true, }); }); @@ -94,8 +89,7 @@ add_task(async function testQueryParamIsStrippedForSiteSpecific() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - canDisable: false, - menuItemVisible: true, + expectedDisabled: false, }); }); @@ -108,22 +102,7 @@ add_task(async function testQueryParamIsNotStrippedForWrongSiteSpecific() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: 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, + expectedDisabled: true, }); }); @@ -136,13 +115,12 @@ add_task(async function testMagneticLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - canDisable: true, - menuItemVisible: false, + expectedDisabled: true, }); }); // Ensuring clean copy is disabled on about links -add_task(async function testMagneticLinks() { +add_task(async function testAboutLinks() { let validUrl = "about:blank"; let shortenedUrl = "about:blank"; await testStripOnShare({ @@ -150,22 +128,20 @@ add_task(async function testMagneticLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - canDisable: true, - menuItemVisible: false, + expectedDisabled: true, }); }); -// Ensure clean copy is diabled when nothing can be stripped +// Ensure clean copy is disabled 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, - canDisable: true, - menuItemVisible: false, + expectedDisabled: true, }); }); @@ -181,65 +157,32 @@ add_task(async function testErrorHandlingForNestedLinks() { strippedURI: shortenedUrl, prefEnabled: true, useTestList: true, - 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, + expectedDisabled: false, }); }); /** - * Opens a new tab, opens the context menu and checks that the strip-on-share menu item is visible. + * Opens a new tab, opens the context menu and checks the menu item. * Checks that the stripped version of the url is copied to the clipboard. * - * @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 + * @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. */ async function testStripOnShare({ originalURI, strippedURI, prefEnabled, useTestList, - canDisable, - menuItemVisible, + expectedDisabled, }) { 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], ], }); @@ -287,29 +230,25 @@ 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" ); - let stripOnShare = contextMenu.querySelector("#context-stripOnShareLink"); - if (menuItemVisible) { - Assert.ok( - BrowserTestUtils.isVisible(stripOnShare), - "Menu item is visible" - ); + if (prefEnabled) { // Make sure the stripped link will be copied to the clipboard await SimpleTest.promiseClipboardChange(strippedURI, () => { - contextMenu.activateItem(stripOnShare); + contextMenu.activateItem(menuItem); }); } 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,7 +13,6 @@ 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,13 +73,6 @@ 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" }) ); @@ -3878,25 +3871,15 @@ export class UrlbarInput { } let controller = this.document.commandDispatcher.getControllerForCommand("cmd_copy"); - // url bar is empty - if (!controller.isCommandEnabled("cmd_copy")) { - stripOnShare.setAttribute("hidden", true); - return; - } - // selection is not a valid url - if (!this.#isClipboardURIValid()) { + if ( + !controller.isCommandEnabled("cmd_copy") || + !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.removeAttribute("disabled"); + stripOnShare.toggleAttribute("disabled", !this.#canStrip()); }); } 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,7 +12,6 @@ add_setup(async function () { set: [ ["privacy.query_stripping.strip_list", "stripParam"], ["privacy.query_stripping.enabled", false], - ["privacy.query_stripping.strip_on_share.canDisable", false], ], }); @@ -26,22 +25,20 @@ add_setup(async function () { // Selection is not a valid URI, menu item should be hidden add_task(async function testInvalidURI() { - await testMenuItemDisabled( - "https://www.example.com/?stripParam=1234", - true, - false, - true - ); + await testMenuItemDisabled({ + url: "https://www.example.com/?stripParam=1234", + prefEnabled: true, + selection: true, + }); }); // Pref is not enabled, menu item should be hidden add_task(async function testPrefDisabled() { - await testMenuItemDisabled( - "https://www.example.com/?stripParam=1234", - false, - false, - false - ); + await testMenuItemDisabled({ + url: "https://www.example.com/?stripParam=1234", + prefEnabled: false, + selection: false, + }); }); // Menu item should be visible, the whole url is copied without a selection, url should be stripped. @@ -53,8 +50,7 @@ add_task(async function testQueryParamIsStripped() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - canDisable: false, - isDisabled: false, + expectedDisabled: false, }); }); @@ -67,28 +63,12 @@ add_task(async function testQueryParamIsStrippedSelectURL() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: 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, + expectedDisabled: false, }); }); // Make sure other parameters don't interfere with stripping -add_task(async function testQueryParamIsStrippedWithDisabletPref() { +add_task(async function testQueryParamIsStrippedWithOtherParam() { let originalUrl = "https://www.example.com/?keepParameter=1&stripParam=1234"; let shortenedUrl = "https://www.example.com/?keepParameter=1"; await testMenuItemEnabled({ @@ -96,20 +76,18 @@ add_task(async function testQueryParamIsStrippedWithDisabletPref() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - canDisable: true, - isDisabled: false, + expectedDisabled: false, }); }); // Test that menu item becomes visible again after selecting a non-url -add_task(async function testQueryParamIsStrippedWithDisabletPref() { +add_task(async function testQueryParamIsStrippedAfterInvalid() { // Selection is not a valid URI, menu item should be hidden - await testMenuItemDisabled( - "https://www.example.com/?stripParam=1234", - true, - true, - true - ); + await testMenuItemDisabled({ + url: "https://www.example.com/?stripParam=1234", + prefEnabled: true, + selection: 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/"; @@ -118,12 +96,11 @@ add_task(async function testQueryParamIsStrippedWithDisabletPref() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - canDisable: true, - isDisabled: false, + expectedDisabled: false, }); }); -// Menu item should be visible, selecting the whole url, url should be the same. +// Menu item should be disabled if url is the same. add_task(async function testURLIsCopiedWithNoParams() { let originalUrl = "https://www.example.com/"; let shortenedUrl = "https://www.example.com/"; @@ -132,22 +109,7 @@ add_task(async function testURLIsCopiedWithNoParams() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: false, - 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, + expectedDisabled: true, }); }); @@ -160,8 +122,7 @@ add_task(async function testQueryParamIsStrippedForSiteSpecific() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - canDisable: false, - isDisabled: false, + expectedDisabled: false, }); }); @@ -174,8 +135,7 @@ add_task(async function testQueryParamIsNotStrippedForWrongSiteSpecific() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - canDisable: false, - isDisabled: false, + expectedDisabled: true, }); }); @@ -189,14 +149,13 @@ add_task(async function testQueryParamIsStrippedWhenParamIsCapitalized() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - canDisable: false, - isDisabled: false, + expectedDisabled: false, }); }); // Ensuring site specific parameters are stripped regardless // of capitalization in the URI -add_task(async function testQueryParamIsStrippedWhenParamIsCapitalized() { +add_task(async function testQueryParamIsStrippedWhenParamIsLowercase() { let originalUrl = "https://www.example.com/?test_5=1234"; let shortenedUrl = "https://www.example.com/"; await testMenuItemEnabled({ @@ -204,25 +163,21 @@ add_task(async function testQueryParamIsStrippedWhenParamIsCapitalized() { validUrl: originalUrl, strippedUrl: shortenedUrl, useTestList: true, - canDisable: false, - isDisabled: false, + expectedDisabled: false, }); }); /** - * Opens a new tab, opens the url bar context menu and checks that the strip-on-share menu item is not visible + * Opens a new tab and checks menu item is hidden in the url bar context menu. * - * @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 + * @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. */ -async function testMenuItemDisabled(url, prefEnabled, canDisable, selection) { +async function testMenuItemDisabled({ url, prefEnabled, selection }) { await SpecialPowers.pushPrefEnv({ - set: [ - ["privacy.query_stripping.strip_on_share.enabled", prefEnabled], - ["privacy.query_stripping.strip_on_share.canDisable", canDisable], - ], + set: [["privacy.query_stripping.strip_on_share.enabled", prefEnabled]], }); await BrowserTestUtils.withNewTab(url, async function () { @@ -247,30 +202,27 @@ async function testMenuItemDisabled(url, prefEnabled, canDisable, selection) { } /** - * Opens a new tab, opens the url bar context menu and checks that the strip-on-share menu item is visible. + * Opens a new tab and checks menu item is visible in the url bar context menu. * Checks that the stripped version of the url is copied to the clipboard. * - * @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 + * @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. */ async function testMenuItemEnabled({ selectWholeUrl, validUrl, strippedUrl, useTestList, - canDisable, - isDisabled, + expectedDisabled, }) { 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], ], }); @@ -298,13 +250,10 @@ async function testMenuItemEnabled({ if (selectWholeUrl) { gURLBar.select(); } + let menuitem = await promiseContextualMenuitem("strip-on-share"); Assert.ok(BrowserTestUtils.isVisible(menuitem), "Menu item is visible"); - if (isDisabled) { - Assert.ok(menuitem.getAttribute("disabled"), "Menu item is greyed out"); - } else { - Assert.ok(!menuitem.getAttribute("disabled"), "Menu item is interactive"); - } + Assert.equal(menuitem.disabled, expectedDisabled, "Menu item is disabled"); 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,7 +14,6 @@ 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,9 +4117,6 @@ 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);