Bug 1571161 - Modify openViewOnFocus so that it doesn't open the view on the newtab page and in private windows. r=dao
authorDrew Willcoxon <adw@mozilla.com>
Thu, 15 Aug 2019 11:09:28 +0000
changeset 488262 31f858085f7cbfccf75359c3a583f5b27a33e50a
parent 488261 067d47e20a9439ad81d0b119b1b660c9638ff08e
child 488263 0f32ec38b8ff3c28ab0312d5503f3668105dfbeb
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1571161
milestone70.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 1571161 - Modify openViewOnFocus so that it doesn't open the view on the newtab page and in private windows. r=dao Differential Revision: https://phabricator.services.mozilla.com/D40719
browser/components/urlbar/UrlbarInput.jsm
browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
--- a/browser/components/urlbar/UrlbarInput.jsm
+++ b/browser/components/urlbar/UrlbarInput.jsm
@@ -837,16 +837,26 @@ class UrlbarInput {
   set value(val) {
     return this._setValue(val, true);
   }
 
   get openViewOnFocus() {
     return this._openViewOnFocus;
   }
 
+  get openViewOnFocusForCurrentTab() {
+    return (
+      this.openViewOnFocus &&
+      !["about:newtab", "about:home"].includes(
+        this.window.gBrowser.currentURI.spec
+      ) &&
+      !this.isPrivate
+    );
+  }
+
   // Private methods below.
 
   _setOpenViewOnFocus() {
     // FIXME: Not using UrlbarPrefs because its pref observer may run after
     // this call, so we'd get the previous openViewOnFocus value here. This
     // can be cleaned up after bug 1560013.
     this._openViewOnFocus = Services.prefs.getBoolPref(
       "browser.urlbar.openViewOnFocus"
@@ -1519,17 +1529,17 @@ class UrlbarInput {
       // The rest of this handler only cares about left clicks.
       if (event.button != 0) {
         return;
       }
 
       if (event.detail == 2 && UrlbarPrefs.get("doubleClickSelectsAll")) {
         this.editor.selectAll();
         event.preventDefault();
-      } else if (this.openViewOnFocus && !this.view.isOpen) {
+      } else if (this.openViewOnFocusForCurrentTab && !this.view.isOpen) {
         this.controller.engagementEvent.start(event);
         this.startQuery({
           allowAutofill: false,
         });
       }
       return;
     }
 
--- a/browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
+++ b/browser/components/urlbar/tests/browser/browser_openViewOnFocus.js
@@ -1,41 +1,129 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-add_task(async function test() {
+async function checkOpensOnFocus(win = window) {
+  Assert.ok(
+    win.gURLBar.openViewOnFocusForCurrentTab,
+    "openViewOnFocusForCurrentTab should be true"
+  );
+  // 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();
+  // Focus with the mouse.
+  await UrlbarTestUtils.promisePopupOpen(win, () => {
+    EventUtils.synthesizeMouseAtCenter(win.gURLBar.inputField, {});
+  });
+  win.gURLBar.blur();
+}
+
+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();
+  Assert.ok(!win.gURLBar.view.isOpen, "check urlbar panel is not open");
+  win.gURLBar.blur();
+  // Focus with the mouse.
+  EventUtils.synthesizeMouseAtCenter(win.gURLBar.inputField, {});
+  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]],
   });
-
   // Add some history for the empty panel.
   await PlacesTestUtils.addVisits([
     {
       uri: "http://mochi.test:8888/",
       transition: PlacesUtils.history.TRANSITIONS.TYPED,
     },
   ]);
   registerCleanupFunction(() => PlacesUtils.history.clear());
+});
 
+add_task(async function test() {
   await BrowserTestUtils.withNewTab(
     { gBrowser, url: "about:blank" },
     async browser => {
-      gURLBar.blur();
-      gURLBar.focus();
-      Assert.ok(!gURLBar.view.isOpen, "check urlbar panel is not open");
-      Assert.ok(gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
-      gURLBar.blur();
-      Assert.ok(gURLBar.dropmarker.hidden, "The dropmarker should be hidden");
-      // Check the keyboard shortcut.
-      await UrlbarTestUtils.promisePopupOpen(window, () => {
-        window.document.getElementById("Browser:OpenLocation").doCommand();
-      });
-      gURLBar.blur();
-      // Focus with the mouse.
-      await UrlbarTestUtils.promisePopupOpen(window, () => {
-        EventUtils.synthesizeMouseAtCenter(gURLBar.inputField, {});
-      });
-      gURLBar.blur();
+      await checkOpensOnFocus();
     }
   );
 });
+
+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 => {
+        // openViewOnFocus should be disabled for these pages even though the
+        // pref is true.
+        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();
+            // 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();
+        // 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();
+      }
+    );
+  }
+});
+
+add_task(async function privateWindow() {
+  let privateWin = await BrowserTestUtils.openNewBrowserWindow({
+    private: true,
+  });
+  checkDoesNotOpenOnFocus(privateWin);
+  await BrowserTestUtils.closeWindow(privateWin);
+});