From bdad30d68ba2b2a70eb2e5fb143f4020d4c50c12 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Fri, 21 Dec 2018 16:58:47 +0000 Subject: [PATCH] Bug 1500486 - QuantumBar: Handle loading of non-urls entered in the new address bar. r=dao Differential Revision: https://phabricator.services.mozilla.com/D10346 --HG-- extra : moz-landing-system : lando --- browser/components/urlbar/UrlbarInput.jsm | 28 ++++---- .../urlbar/tests/browser/browser.ini | 1 + .../tests/browser/browser_UrlbarLoadRace.js | 72 +++++++++++++++++++ .../components/urlbar/tests/browser/head.js | 1 + 4 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index a4c4c447cdb3..9a0612e141dc 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -234,23 +234,21 @@ class UrlbarInput { try { new URL(url); } catch (ex) { - // TODO: Figure out why we need lastLocationChange here. - // let lastLocationChange = browser.lastLocationChange; - // UrlbarUtils.getShortcutOrURIAndPostData(text).then(data => { - // if (where != "current" || - // browser.lastLocationChange == lastLocationChange) { - // params.postData = data.postData; - // params.allowInheritPrincipal = data.mayInheritPrincipal; - // this._loadURL(data.url, browser, where, - // openUILinkParams); - // } - // }); + let browser = this.window.gBrowser.selectedBrowser; + let lastLocationChange = browser.lastLocationChange; + + UrlbarUtils.getShortcutOrURIAndPostData(url).then(data => { + if (where != "current" || + browser.lastLocationChange == lastLocationChange) { + openParams.postData = data.postData; + openParams.allowInheritPrincipal = data.mayInheritPrincipal; + this._loadURL(data.url, where, openParams); + } + }); return; } this._loadURL(url, where, openParams); - - this.view.close(); } /** @@ -560,7 +558,7 @@ class UrlbarInput { browser.initialPageLoadedFromURLBar = url; } try { - UrlbarUtils.addToUrlbarHistory(url); + UrlbarUtils.addToUrlbarHistory(url, this.window); } catch (ex) { // Things may go wrong when adding url to session history, // but don't let that interfere with the loading of the url. @@ -602,6 +600,8 @@ class UrlbarInput { // TODO This should probably be handed via input. // Ensure the start of the URL is visible for usability reasons. // this.selectionStart = this.selectionEnd = 0; + + this.closePopup(); } /** diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index d27448db7dd0..f0fa11ca770e 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -13,3 +13,4 @@ support-files = subsuite = clipboard [browser_UrlbarInput_unit.js] support-files = empty.xul +[browser_UrlbarLoadRace.js] diff --git a/browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js b/browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js new file mode 100644 index 000000000000..6d44a6b06e68 --- /dev/null +++ b/browser/components/urlbar/tests/browser/browser_UrlbarLoadRace.js @@ -0,0 +1,72 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +// This test is for testing races of loading the Urlbar when loading shortcuts. +// For example, ensuring that if a search query is entered, but something causes +// a page load whilst we're getting the search url, then we don't handle the +// original search query. + +add_task(async function setup() { + sandbox = sinon.sandbox.create(); + + registerCleanupFunction(async () => { + sandbox.restore(); + }); +}); + +async function checkShortcutLoading(modifierKeys) { + let deferred = PromiseUtils.defer(); + let tab = await BrowserTestUtils.openNewForegroundTab({ + gBrowser, + opening: "about:robots", + }); + + // We stub getShortcutOrURIAndPostData so that we can guarentee it doesn't resolve + // until after we've loaded a new page. + sandbox.stub(UrlbarUtils, "getShortcutOrURIAndPostData").callsFake(() => deferred.promise); + + gURLBar.focus(); + gURLBar.value = "search"; + gURLBar.userTypedValue = true; + EventUtils.synthesizeKey("KEY_Enter", modifierKeys); + + Assert.ok(UrlbarUtils.getShortcutOrURIAndPostData.calledOnce, + "should have called getShortcutOrURIAndPostData"); + + BrowserTestUtils.loadURI(tab.linkedBrowser, "about:license"); + await BrowserTestUtils.browserLoaded(tab.linkedBrowser); + + let openedTab; + function listener(event) { + openedTab = event.target; + } + window.addEventListener("TabOpen", listener); + + deferred.resolve({ + url: "https://example.com/1/", + postData: {}, + mayInheritPrincipal: true, + }); + + await deferred.promise; + + if (modifierKeys) { + Assert.ok(openedTab, "Should have attempted to open the shortcut page"); + BrowserTestUtils.removeTab(openedTab); + } else { + Assert.ok(!openedTab, "Should have not attempted to open the shortcut page"); + } + + window.removeEventListener("TabOpen", listener); + BrowserTestUtils.removeTab(tab); + sandbox.restore(); +} + +add_task(async function test_location_change_stops_load() { + await checkShortcutLoading(); +}); + +add_task(async function test_opening_different_tab_with_location_change() { + await checkShortcutLoading({altKey: true}); +}); diff --git a/browser/components/urlbar/tests/browser/head.js b/browser/components/urlbar/tests/browser/head.js index c08a7e641be7..5d7db484aa1c 100644 --- a/browser/components/urlbar/tests/browser/head.js +++ b/browser/components/urlbar/tests/browser/head.js @@ -11,6 +11,7 @@ let sandbox; ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetters(this, { + PromiseUtils: "resource://gre/modules/PromiseUtils.jsm", Services: "resource://gre/modules/Services.jsm", QueryContext: "resource:///modules/UrlbarUtils.jsm", UrlbarController: "resource:///modules/UrlbarController.jsm",