Bug 1485746 - Cursor gets reset to start of address bar on window switch. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 30 Aug 2018 09:41:24 +0000
changeset 434044 621979b26f697513f06f4c5f2db1d5ee580c4363
parent 434043 7d47dca5962e1dc0b3426814885f9a5bd27afb07
child 434045 9550be79a88ef8807582722835275c4d20c83a4a
push id68523
push usermak77@bonardo.net
push dateThu, 30 Aug 2018 09:43:18 +0000
treeherderautoland@621979b26f69 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1485746
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1485746 - Cursor gets reset to start of address bar on window switch. r=adw This restores the previous behavior where we set the selection only when setting a new different value Differential Revision: https://phabricator.services.mozilla.com/D4528
browser/base/content/browser.js
browser/base/content/test/urlbar/browser.ini
browser/base/content/test/urlbar/browser_autocomplete_cursor.js
browser/base/content/test/urlbar/browser_urlOverflow.js
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2681,19 +2681,25 @@ function URLBarSetURI(aURI) {
 
     valid = !isBlankPageURL(uri.spec) || uri.schemeIs("moz-extension");
   } else if (isInitialPage(value) &&
              checkEmptyPageOrigin(gBrowser.selectedBrowser)) {
     value = "";
     valid = true;
   }
 
+  let isDifferentValidValue = valid && value != gURLBar.value;
   gURLBar.value = value;
   gURLBar.valueIsTyped = !valid;
   gURLBar.removeAttribute("usertyping");
+  if (isDifferentValidValue) {
+    // The selection is enforced only for new values, to avoid overriding the
+    // cursor position when the user switches windows while typing.
+    gURLBar.selectionStart = gURLBar.selectionEnd = 0;
+  }
 
   SetPageProxyState(valid ? "valid" : "invalid");
 }
 
 function losslessDecodeURI(aURI) {
   let scheme = aURI.scheme;
   if (scheme == "moz-action")
     throw new Error("losslessDecodeURI should never get a moz-action URI");
--- a/browser/base/content/test/urlbar/browser.ini
+++ b/browser/base/content/test/urlbar/browser.ini
@@ -146,17 +146,16 @@ support-files =
 [browser_urlbar_search_no_speculative_connect_with_client_cert.js]
 [browser_urlbar_stop_pending.js]
 support-files =
   slow-page.sjs
 [browser_urlbar_remoteness_switch.js]
 run-if = e10s
 [browser_urlHighlight.js]
 [browser_urlOverflow.js]
-skip-if = true # Bug 1482557
 [browser_wyciwyg_urlbarCopying.js]
 subsuite = clipboard
 support-files =
   test_wyciwyg_copying.html
 [browser_urlbarStopSearchOnSelection.js]
 support-files =
   searchSuggestionEngineSlow.xml
   searchSuggestionEngine.sjs
--- a/browser/base/content/test/urlbar/browser_autocomplete_cursor.js
+++ b/browser/base/content/test/urlbar/browser_autocomplete_cursor.js
@@ -1,12 +1,12 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-add_task(async function() {
+add_task(async function test_arrowRight() {
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:mozilla");
   await promiseAutocompleteResultPopup("www.mozilla.org");
   await waitForAutocompleteResultAt(0);
 
   gURLBar.selectTextRange(4, 4);
 
   is(gURLBar.popup.state, "open", "Popup should be open");
   is(gURLBar.popup.richlistbox.selectedIndex, 0, "Should have selected something");
@@ -14,8 +14,27 @@ add_task(async function() {
   EventUtils.synthesizeKey("KEY_ArrowRight");
   await promisePopupHidden(gURLBar.popup);
 
   is(gURLBar.selectionStart, 5, "Should have moved the cursor");
   is(gURLBar.selectionEnd, 5, "And not selected anything");
 
   BrowserTestUtils.removeTab(tab);
 });
+
+add_task(async function test_windowSwitch() {
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:mozilla");
+  await promiseAutocompleteResultPopup("www.mozilla.org");
+  await waitForAutocompleteResultAt(0);
+
+  gURLBar.focus();
+  gURLBar.selectTextRange(4, 4);
+
+  let newWindow = await BrowserTestUtils.openNewBrowserWindow();
+
+  await BrowserTestUtils.closeWindow(newWindow);
+
+  Assert.equal(document.activeElement, gURLBar.inputField, "URL Bar should be focused");
+  is(gURLBar.selectionStart, 4, "Should not have moved the cursor");
+  is(gURLBar.selectionEnd, 4, "Should not have selected anything");
+
+  BrowserTestUtils.removeTab(tab);
+});
--- a/browser/base/content/test/urlbar/browser_urlOverflow.js
+++ b/browser/base/content/test/urlbar/browser_urlOverflow.js
@@ -1,51 +1,58 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 async function testVal(aExpected, overflowSide = "") {
   info(`Testing ${aExpected}`);
-  gURLBar.value = aExpected;
+  URLBarSetURI(makeURI(aExpected));
+
+  Assert.equal(gURLBar.selectionStart, gURLBar.selectionEnd,
+    "Selection sanity check");
 
   gURLBar.focus();
+  Assert.equal(document.activeElement, gURLBar.inputField, "URL Bar should be focused");
   Assert.equal(gURLBar.scheme.value, "", "Check the scheme value");
   Assert.equal(getComputedStyle(gURLBar.scheme).visibility, "hidden",
                "Check the scheme box visibility");
 
   gURLBar.blur();
   await window.promiseDocumentFlushed(() => {});
   // The attribute doesn't always change, so we can't use waitForAttribute.
   await TestUtils.waitForCondition(
     () => gURLBar.getAttribute("textoverflow") === overflowSide);
 
   let scheme = aExpected.match(/^([a-z]+:\/{0,2})/)[1];
   // We strip http, so we should not show the scheme for it.
   if (scheme == "http://" && Services.prefs.getBoolPref("browser.urlbar.trimURLs", true))
     scheme = "";
 
-  Assert.equal(gURLBar.selectionStart, gURLBar.selectionEnd,
-               "Selection sanity check");
   Assert.equal(gURLBar.scheme.value, scheme, "Check the scheme value");
   let isOverflowed = gURLBar.inputField.scrollWidth > gURLBar.inputField.clientWidth;
   Assert.equal(isOverflowed, !!overflowSide, "Check The input field overflow");
   Assert.equal(gURLBar.getAttribute("textoverflow"), overflowSide,
                "Check the textoverflow attribute");
   if (overflowSide) {
     let side = gURLBar.inputField.scrollLeft == 0 ? "end" : "start";
     Assert.equal(side, overflowSide, "Check the overflow side");
     Assert.equal(getComputedStyle(gURLBar.scheme).visibility,
                  scheme && isOverflowed && overflowSide == "start" ? "visible" : "hidden",
                  "Check the scheme box visibility");
   }
 }
 
 add_task(async function() {
+  // We use a new tab for the test to be sure all the tab switching and loading
+  // is complete before starting, otherwise onLocationChange for this tab could
+  // override the value we set with an empty value.
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
   registerCleanupFunction(function() {
     URLBarSetURI();
+    BrowserTestUtils.removeTab(tab);
   });
 
   let lotsOfSpaces = new Array(200).fill("%20").join("");
 
   // اسماء.شبكة
   let rtlDomain = "\u0627\u0633\u0645\u0627\u0621\u002e\u0634\u0628\u0643\u0629";
 
   // Mix the direction of the tests to cover more cases, and to ensure the
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -593,19 +593,19 @@ file, You can obtain one at http://mozil
           // We strip http, so we should not show the scheme box for it.
           if (!this._mayTrimURLs || schemeWSlashes != "http://") {
             this.scheme.value = schemeWSlashes;
             this.inputField.style.setProperty("--urlbar-scheme-size",
                                               schemeWSlashes.length + "ch");
           }
 
           // Make sure the host is always visible. Since it is aligned on
-          // the first strong directional character, we set the overflow
-          // appropriately.
-          this.selectionStart = this.selectionEnd = 0;
+          // the first strong directional character, we set scrollLeft
+          // appropriately to ensure the domain stays visible in case of an
+          // overflow.
           window.requestAnimationFrame(() => {
             // Check for re-entrance. On focus change this formatting code is
             // invoked regardless, thus this should be enough.
             if (this._formattingInstance != instance)
               return;
             let directionality = window.windowUtils.getDirectionFromText(domain);
             // In the future, for example in bug 525831, we may add a forceRTL
             // char just after the domain, and in such a case we should not
@@ -975,16 +975,21 @@ file, You can obtain one at http://mozil
             openTrustedLinkIn(url, openUILinkWhere, params);
           } catch (ex) {
             // This load can throw an exception in certain cases, which means
             // we'll want to replace the URL with the loaded URL:
             if (ex.result != Cr.NS_ERROR_LOAD_SHOWED_ERRORPAGE) {
               this.handleRevert();
             }
           }
+
+          if (openUILinkWhere == "current") {
+            // Ensure the start of the URL is visible for usability reasons.
+            this.selectionStart = this.selectionEnd = 0;
+          }
         ]]></body>
       </method>
 
       <method name="_parseAndRecordSearchEngineLoad">
         <parameter name="engineOrEngineName"/>
         <parameter name="query"/>
         <parameter name="event"/>
         <parameter name="openUILinkWhere"/>