From 784bf6634e7e925f2c9a5d2ab333ec9174ac99d9 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Mon, 2 Mar 2020 18:34:17 +0000 Subject: [PATCH] Bug 333714 - Unify clickSelectsAll behavior across all platforms. r=dao Differential Revision: https://phabricator.services.mozilla.com/D64783 --HG-- extra : moz-landing-system : lando --- browser/app/profile/firefox.js | 11 --- .../components/search/content/searchbar.js | 10 +-- browser/components/urlbar/UrlbarInput.jsm | 20 ++--- browser/components/urlbar/UrlbarPrefs.jsm | 10 --- .../urlbar/tests/browser/browser.ini | 1 - .../browser/browser_doubleClickSelectsAll.js | 45 ----------- .../browser/browser_retainedResultsOnFocus.js | 1 - .../tests/browser/browser_urlbar_selection.js | 74 +++++++++++-------- modules/libpref/init/all.js | 4 - 9 files changed, 52 insertions(+), 124 deletions(-) delete mode 100644 browser/components/urlbar/tests/browser/browser_doubleClickSelectsAll.js diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index bddb330e4e8f..343d27b30e3c 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -251,17 +251,6 @@ pref("browser.warnOnQuit", true); pref("browser.fullscreen.autohide", true); pref("browser.overlink-delay", 80); -#ifdef UNIX_BUT_NOT_MAC - pref("browser.urlbar.clickSelectsAll", false); -#else - pref("browser.urlbar.clickSelectsAll", true); -#endif -#ifdef UNIX_BUT_NOT_MAC - pref("browser.urlbar.doubleClickSelectsAll", true); -#else - pref("browser.urlbar.doubleClickSelectsAll", false); -#endif - // Whether using `ctrl` when hitting return/enter in the URL bar // (or clicking 'go') should prefix 'www.' and suffix // browser.fixup.alternate.suffix to the URL bar value prior to diff --git a/browser/components/search/content/searchbar.js b/browser/components/search/content/searchbar.js index 0be65d690788..876bc918d572 100644 --- a/browser/components/search/content/searchbar.js +++ b/browser/components/search/content/searchbar.js @@ -433,16 +433,15 @@ /** * Determines if we should select all the text in the searchbar based on the - * clickSelectsAll pref, searchbar state, and whether the selection is empty. + * searchbar state, and whether the selection is empty. */ _maybeSelectAll() { if ( !this._preventClickSelectsAll && - UrlbarPrefs.get("clickSelectsAll") && document.activeElement == this._textbox && this._textbox.selectionStart == this._textbox.selectionEnd ) { - this._textbox.editor.selectAll(); + this.select(); } } @@ -564,11 +563,6 @@ // is text in the textbox. this.openSuggestionsPanel(true); } - - if (event.detail == 2 && UrlbarPrefs.get("doubleClickSelectsAll")) { - this._textbox.editor.selectAll(); - event.preventDefault(); - } }); } diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index c649dd51accb..ec917c1d643e 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -1692,19 +1692,16 @@ class UrlbarInput { /** * Determines if we should select all the text in the Urlbar based on the - * clickSelectsAll pref, Urlbar state, and whether the selection is empty. - * @param {boolean} [ignoreClickSelectsAllPref] - * If true, the browser.urlbar.clickSelectsAll pref will be ignored. + * Urlbar state, and whether the selection is empty. */ - _maybeSelectAll(ignoreClickSelectsAllPref = false) { + _maybeSelectAll() { if ( !this._preventClickSelectsAll && - (ignoreClickSelectsAllPref || UrlbarPrefs.get("clickSelectsAll")) && this._compositionState != UrlbarUtils.COMPOSITION.COMPOSING && this.document.activeElement == this.inputField && this.inputField.selectionStart == this.inputField.selectionEnd ) { - this.editor.selectAll(); + this.select(); } } @@ -1785,9 +1782,7 @@ class UrlbarInput { return; } - // If the user right clicks, we select all regardless of the value of - // the browser.urlbar.clickSelectsAll pref. - this._maybeSelectAll(/* ignoreClickSelectsAllPref */ event.button == 2); + this._maybeSelectAll(); } _on_focus(event) { @@ -1800,7 +1795,7 @@ class UrlbarInput { if (this.focusedViaMousedown) { this.view.autoOpen({ event }); } else if (this.inputField.hasAttribute("refocused-by-panel")) { - this._maybeSelectAll(true); + this._maybeSelectAll(); } this._updateUrlTooltip(); @@ -1849,10 +1844,7 @@ class UrlbarInput { this.selectionStart = this.selectionEnd = 0; } - if (event.detail == 2 && UrlbarPrefs.get("doubleClickSelectsAll")) { - this.editor.selectAll(); - event.preventDefault(); - } else if (event.target.id == SEARCH_BUTTON_ID) { + if (event.target.id == SEARCH_BUTTON_ID) { this._preventClickSelectsAll = true; this.search(UrlbarTokenizer.RESTRICT.SEARCH); } else { diff --git a/browser/components/urlbar/UrlbarPrefs.jsm b/browser/components/urlbar/UrlbarPrefs.jsm index 8705367f1c1e..7552ea53aa68 100644 --- a/browser/components/urlbar/UrlbarPrefs.jsm +++ b/browser/components/urlbar/UrlbarPrefs.jsm @@ -41,11 +41,6 @@ const PREF_URLBAR_DEFAULTS = new Map([ // this value. See UnifiedComplete. ["autoFill.stddevMultiplier", [0.0, "getFloatPref"]], - // If true, this optimizes for replacing the full URL rather than editing - // part of it. This also copies the urlbar value to the selection clipboard - // on systems that support it. - ["clickSelectsAll", false], - // Whether using `ctrl` when hitting return/enter in the URL bar // (or clicking 'go') should prefix 'www.' and suffix // browser.fixup.alternate.suffix to the URL bar value prior to @@ -66,11 +61,6 @@ const PREF_URLBAR_DEFAULTS = new Map([ // but this would mean flushing layout.) ["disableExtendForTests", false], - // If true, this optimizes for replacing the full URL rather than selecting a - // portion of it. This also copies the urlbar value to the selection - // clipboard on systems that support it. - ["doubleClickSelectsAll", false], - // Whether telemetry events should be recorded. ["eventTelemetry.enabled", false], diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index 35f46f714eff..00de23ac17ef 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -46,7 +46,6 @@ support-files = [browser_deleteAllText.js] [browser_dns_first_for_single_words.js] skip-if = verify && os == 'linux' # Bug 1581635 -[browser_doubleClickSelectsAll.js] [browser_downArrowKeySearch.js] [browser_dragdropURL.js] [browser_dropmarker.js] diff --git a/browser/components/urlbar/tests/browser/browser_doubleClickSelectsAll.js b/browser/components/urlbar/tests/browser/browser_doubleClickSelectsAll.js deleted file mode 100644 index a52d8437a8eb..000000000000 --- a/browser/components/urlbar/tests/browser/browser_doubleClickSelectsAll.js +++ /dev/null @@ -1,45 +0,0 @@ -/* 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 doubleClick(target) { - let promise = BrowserTestUtils.waitForEvent(target, "dblclick"); - EventUtils.synthesizeMouseAtCenter( - target, - { clickCount: 1 }, - target.ownerGlobal - ); - EventUtils.synthesizeMouseAtCenter( - target, - { clickCount: 2 }, - target.ownerGlobal - ); - return promise; -} - -add_task(async function() { - await SpecialPowers.pushPrefEnv({ - set: [ - ["browser.urlbar.clickSelectsAll", false], - ["browser.urlbar.doubleClickSelectsAll", true], - ], - }); - - let url = "about:mozilla"; - let win = await BrowserTestUtils.openNewBrowserWindow(); - await BrowserTestUtils.openNewForegroundTab({ gBrowser: win.gBrowser, url }); - - await doubleClick(win.gURLBar.inputField); - is( - win.gURLBar.selectionStart, - 0, - "Selection should start at the beginning of the urlbar value" - ); - is( - win.gURLBar.selectionEnd, - url.length, - "Selection should end at the end of the urlbar value" - ); - - win.close(); -}); diff --git a/browser/components/urlbar/tests/browser/browser_retainedResultsOnFocus.js b/browser/components/urlbar/tests/browser/browser_retainedResultsOnFocus.js index 8cd6657ed75f..084bafa9319c 100644 --- a/browser/components/urlbar/tests/browser/browser_retainedResultsOnFocus.js +++ b/browser/components/urlbar/tests/browser/browser_retainedResultsOnFocus.js @@ -73,7 +73,6 @@ add_task(async function setup() { await SpecialPowers.pushPrefEnv({ set: [ ["browser.urlbar.autoFill", true], - ["browser.urlbar.clickSelectsAll", true], ["browser.urlbar.openViewOnFocus", true], ], }); diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_selection.js b/browser/components/urlbar/tests/browser/browser_urlbar_selection.js index d8841d53ab33..571333ce116e 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_selection.js +++ b/browser/components/urlbar/tests/browser/browser_urlbar_selection.js @@ -66,11 +66,27 @@ function drag(target, fromX, fromY, toX, toY) { return promise; } -add_task(async function setup() { - SpecialPowers.pushPrefEnv({ - set: [["browser.urlbar.clickSelectsAll", true]], - }); +function resetPrimarySelection(val = "") { + if (Services.clipboard.supportsSelectionClipboard()) { + // Reset the clipboard. + clipboardHelper.copyStringToClipboard( + val, + Services.clipboard.kSelectionClipboard + ); + } +} +function checkPrimarySelection(expectedVal = "") { + if (Services.clipboard.supportsSelectionClipboard()) { + let primaryAsText = SpecialPowers.getClipboardData( + "text/unicode", + SpecialPowers.Ci.nsIClipboard.kSelectionClipboard + ); + Assert.equal(primaryAsText, expectedVal); + } +} + +add_task(async function setup() { // On macOS, we must "warm up" the Urlbar to get the first test to pass. gURLBar.value = ""; await click(gURLBar.inputField); @@ -78,6 +94,7 @@ add_task(async function setup() { }); add_task(async function leftClickSelectsAll() { + resetPrimarySelection(); gURLBar.value = exampleSearch; await click(gURLBar.inputField); Assert.equal( @@ -91,9 +108,11 @@ add_task(async function leftClickSelectsAll() { "The entire search term should be selected." ); gURLBar.blur(); + checkPrimarySelection(); }); add_task(async function leftClickSelectsUrl() { + resetPrimarySelection(); gURLBar.value = exampleUrl; await click(gURLBar.inputField); Assert.equal(gURLBar.selectionStart, 0, "The entire url should be selected."); @@ -103,41 +122,18 @@ add_task(async function leftClickSelectsUrl() { "The entire url should be selected." ); gURLBar.blur(); -}); - -// Test to ensure that the doubleClickSelectsAll pref does not interfere with -// single click behaviour (Double CSA itself is tested in -// urlbar/tests/browser_doubleClickSelectsAll.js). -add_task(async function bothPrefsEnabled() { - Services.prefs.setBoolPref("browser.urlbar.doubleClickSelectsAll", true); - gURLBar.value = exampleSearch; - await click(gURLBar.inputField); - Assert.equal( - gURLBar.selectionStart, - 0, - "The entire search term should be selected." - ); - Assert.equal( - gURLBar.selectionEnd, - exampleSearch.length, - "The entire search term should be selected." - ); - gURLBar.blur(); - Services.prefs.clearUserPref("browser.urlbar.doubleClickSelectsAll"); + checkPrimarySelection(); }); add_task(async function rightClickSelectsAll() { - // The text should be selected even when the pref is disabled. - await SpecialPowers.pushPrefEnv({ - set: [["browser.urlbar.clickSelectsAll", false]], - }); - gURLBar.inputField.focus(); gURLBar.value = exampleUrl; // Remove the selection so the focus() call above doesn't influence the test. gURLBar.selectionStart = gURLBar.selectionEnd = 0; + resetPrimarySelection(); + await openContextMenu(gURLBar.inputField); Assert.equal(gURLBar.selectionStart, 0, "The entire URL should be selected."); @@ -147,6 +143,8 @@ add_task(async function rightClickSelectsAll() { "The entire URL should be selected." ); + checkPrimarySelection(); + let contextMenu = gURLBar.querySelector("moz-input-box").menupopup; // While the context menu is open, test the "Select All" button. @@ -184,6 +182,7 @@ add_task(async function rightClickSelectsAll() { gURLBar.querySelector("moz-input-box").menupopup.hidePopup(); gURLBar.blur(); + checkPrimarySelection(gURLBar.value); await SpecialPowers.popPrefEnv(); }); @@ -194,6 +193,8 @@ add_task(async function contextMenuDoesNotCancelSelection() { gURLBar.selectionStart = 3; gURLBar.selectionEnd = 7; + resetPrimarySelection(); + await openContextMenu(gURLBar.inputField); Assert.equal( @@ -209,9 +210,11 @@ add_task(async function contextMenuDoesNotCancelSelection() { gURLBar.querySelector("moz-input-box").menupopup.hidePopup(); gURLBar.blur(); + checkPrimarySelection(); }); add_task(async function dragSelect() { + resetPrimarySelection(); gURLBar.value = exampleSearch.repeat(10); // Drags from an artibrary offset of 30 to test for bug 1562145: that the // selection does not start at the beginning. @@ -222,7 +225,12 @@ add_task(async function dragSelect() { "Selection should not start at the beginning of the string." ); + let selectedVal = gURLBar.value.substring( + gURLBar.selectionStart, + gURLBar.selectionEnd + ); gURLBar.blur(); + checkPrimarySelection(selectedVal); }); /** @@ -230,6 +238,7 @@ add_task(async function dragSelect() { * Urlbar is dragged following a selectsAll event then a blur. */ add_task(async function dragAfterSelectAll() { + resetPrimarySelection(); gURLBar.value = exampleSearch.repeat(10); await click(gURLBar.inputField); Assert.equal( @@ -244,6 +253,7 @@ add_task(async function dragAfterSelectAll() { ); gURLBar.blur(); + checkPrimarySelection(); // The offset of 30 is arbitrary. await drag(gURLBar.inputField, 30, 0, 60, 0); @@ -258,6 +268,10 @@ add_task(async function dragAfterSelectAll() { exampleSearch.repeat(10).length, "Only part of the search term should be selected." ); + + checkPrimarySelection( + gURLBar.value.substring(gURLBar.selectionStart, gURLBar.selectionEnd) + ); }); /** diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 3377808eba8d..f46deafefd98 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -3541,8 +3541,6 @@ pref("ui.mouse.radius.inputSource.touchOnly", true); pref("middlemouse.openNewWindow", true); pref("middlemouse.scrollbarPosition", true); - pref("browser.urlbar.clickSelectsAll", false); - // Tab focus model bit field: // 1 focuses text controls, 2 focuses other form elements, 4 adds links. // Leave this at the default, 7, to match mozilla1.0-era user expectations. @@ -3584,8 +3582,6 @@ pref("ui.mouse.radius.inputSource.touchOnly", true); pref("middlemouse.openNewWindow", true); pref("middlemouse.scrollbarPosition", true); - pref("browser.urlbar.clickSelectsAll", false); - // Tab focus model bit field: // 1 focuses text controls, 2 focuses other form elements, 4 adds links. // Leave this at the default, 7, to match mozilla1.0-era user expectations.