Bug 1582505 - OpenViewOnFocus should not open the view on the new tab page using a shortcut. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 19 Sep 2019 21:20:34 +0000
changeset 494246 a511ef7cdbefa8392207b6401e9f880bc53874d4
parent 494245 0add1eb7bd28c84814381fe5ea557f6019918dd5
child 494247 a03583b4aa2b4f749c546043c7aabef76f4afa5c
push id114114
push userdluca@mozilla.com
push dateFri, 20 Sep 2019 22:00:08 +0000
treeherdermozilla-inbound@56e11fddf939 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1582505
milestone71.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 1582505 - OpenViewOnFocus should not open the view on the new tab page using a shortcut. r=adw Differential Revision: https://phabricator.services.mozilla.com/D46491
browser/base/content/browser.js
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2873,17 +2873,17 @@ function focusAndSelectUrlBar() {
   }
 
   gURLBar.select();
 }
 
 function openLocation(event) {
   if (window.location.href == AppConstants.BROWSER_CHROME_URL) {
     focusAndSelectUrlBar();
-    if (gURLBar.openViewOnFocus && !gURLBar.view.isOpen) {
+    if (gURLBar.openViewOnFocusForCurrentTab && !gURLBar.view.isOpen) {
       gURLBar.startQuery({ event });
     }
     return;
   }
 
   // If there's an open browser window, redirect the command there.
   let win = getTopWin();
   if (win) {
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -913,23 +913,19 @@ class UrlbarInput {
   get untrimmedValue() {
     return this._untrimmedValue;
   }
 
   set value(val) {
     return this._setValue(val, true);
   }
 
-  get openViewOnFocus() {
-    return this._openViewOnFocus;
-  }
-
   get openViewOnFocusForCurrentTab() {
     return (
-      this.openViewOnFocus &&
+      this._openViewOnFocus &&
       !["about:newtab", "about:home"].includes(
         this.window.gBrowser.currentURI.spec
       ) &&
       !this.isPrivate
     );
   }
 
   async updateLayoutBreakout() {
--- a/browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
+++ b/browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
@@ -11,46 +11,60 @@ async function checkOpensOnFocus(win = w
   // Even with openViewOnFocus = true, the view should not open when the input
   // is focused programmatically.
   win.gURLBar.blur();
   win.gURLBar.focus();
   Assert.ok(!win.gURLBar.view.isOpen, "check urlbar panel is not open");
   Assert.ok(win.gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
   win.gURLBar.blur();
   Assert.ok(win.gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
+
   // Check the keyboard shortcut.
   await UrlbarTestUtils.promisePopupOpen(win, () => {
     win.document.getElementById("Browser:OpenLocation").doCommand();
   });
-  win.gURLBar.blur();
+  await UrlbarTestUtils.promisePopupClose(win, () => {
+    win.gURLBar.blur();
+  });
+
   // Focus with the mouse.
   await UrlbarTestUtils.promisePopupOpen(win, () => {
     EventUtils.synthesizeMouseAtCenter(win.gURLBar.inputField, {});
   });
-  win.gURLBar.blur();
+  await UrlbarTestUtils.promisePopupClose(win, () => {
+    win.gURLBar.blur();
+  });
 }
 
-function checkDoesNotOpenOnFocus(win = window) {
+async function checkDoesNotOpenOnFocus(win = window) {
   Assert.ok(
     !win.gURLBar.openViewOnFocusForCurrentTab,
     "openViewOnFocusForCurrentTab should be false"
   );
   // The view should not open when the input is focused programmatically.
   win.gURLBar.blur();
   win.gURLBar.focus();
   Assert.ok(!win.gURLBar.view.isOpen, "check urlbar panel is not open");
   Assert.ok(win.gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
   win.gURLBar.blur();
   Assert.ok(win.gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
+
   // Check the keyboard shortcut.
   win.document.getElementById("Browser:OpenLocation").doCommand();
+  // Because the panel opening may not be immediate, we must wait a bit.
+  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+  await new Promise(resolve => setTimeout(resolve, 500));
   Assert.ok(!win.gURLBar.view.isOpen, "check urlbar panel is not open");
   win.gURLBar.blur();
+
   // Focus with the mouse.
   EventUtils.synthesizeMouseAtCenter(win.gURLBar.inputField, {});
+  // Because the panel opening may not be immediate, we must wait a bit.
+  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+  await new Promise(resolve => setTimeout(resolve, 500));
   Assert.ok(!win.gURLBar.view.isOpen, "check urlbar panel is not open");
   win.gURLBar.blur();
 }
 
 add_task(async function setUp() {
   await SpecialPowers.pushPrefEnv({
     set: [["browser.urlbar.openViewOnFocus", true]],
   });
@@ -75,43 +89,48 @@ add_task(async function test() {
 
 add_task(async function newtabAndHome() {
   for (let url of ["about:newtab", "about:home"]) {
     // withNewTab randomly hangs on these pages when waitForLoad = true (the
     // default), so pass false.
     await BrowserTestUtils.withNewTab(
       { gBrowser, url, waitForLoad: false },
       async browser => {
+        // We don't wait for load, but we must ensure to be on the expected url.
+        await TestUtils.waitForCondition(
+          () => window.gBrowser.currentURI.spec == url,
+          "Ensure we're on the expected page"
+        );
         // openViewOnFocus should be disabled for these pages even though the
         // pref is true.
-        checkDoesNotOpenOnFocus();
+        await checkDoesNotOpenOnFocus();
         // Open a new tab where openViewOnFocus should be enabled.
         await BrowserTestUtils.withNewTab(
           { gBrowser, url: "http://example.com/" },
           async otherBrowser => {
             // openViewOnFocus should be enabled.
             await checkOpensOnFocus();
             // Switch back to about:newtab/home.  openViewOnFocus should be
             // disabled.
             await BrowserTestUtils.switchTab(
               gBrowser,
               gBrowser.getTabForBrowser(browser)
             );
-            checkDoesNotOpenOnFocus();
+            await checkDoesNotOpenOnFocus();
             // Switch back to example.com.  openViewOnFocus should be enabled.
             await BrowserTestUtils.switchTab(
               gBrowser,
               gBrowser.getTabForBrowser(otherBrowser)
             );
             await checkOpensOnFocus();
           }
         );
         // After example.com closes, about:newtab/home should be selected again,
         // and openViewOnFocus should be disabled.
-        checkDoesNotOpenOnFocus();
+        await checkDoesNotOpenOnFocus();
         // Load example.com in the same tab.  openViewOnFocus should become
         // enabled.
         await BrowserTestUtils.loadURI(
           gBrowser.selectedBrowser,
           "http://example.com/"
         );
         await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
         await checkOpensOnFocus();
@@ -119,11 +138,11 @@ add_task(async function newtabAndHome() 
     );
   }
 });
 
 add_task(async function privateWindow() {
   let privateWin = await BrowserTestUtils.openNewBrowserWindow({
     private: true,
   });
-  checkDoesNotOpenOnFocus(privateWin);
+  await checkDoesNotOpenOnFocus(privateWin);
   await BrowserTestUtils.closeWindow(privateWin);
 });