Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-21 01:05:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1559264 - Quantumbar: Don't call setValueFromResult when opening the history popup. r=dao

Browse Source 
The problem is that on switching back to the first tab (see the bug), userTypedValue is non-null when URLBarSetURI is called. Therefore the proxy state can't be valid. Something about bug 1529931 caused userTypedValue to go from null to non-null in this case. Details below, but the summary is that we shouldn't be calling UrlbarInput.setValueFromResult when opening the history popup, because setValueFromResult sets userTypedValue.

Before bug 1529931, result.autofill would always be undefined for the first result in the history popup, because we didn't allow UnifiedComplete to return an autofill result for the search triggered by the history popup. After that bug, UnifiedComplete could return an autofill result in that case -- and it likely would since the first result in the history popup has a very high frecency, which also makes it eligible for autofill.

The problem with having an autofill result in the history popup is that it triggers the input.setValueFromResult() call in UrlbarController.receiveResults [1], and setValueFromResult sets userTypedValue. So when the user opens the history popup, userTypedValue gets set to a non-null value (input._lastSearchString).

The fix is to not allow autofill for the history popup. After making that fix on revision https://hg.mozilla.org/mozilla-central/rev/5e2a3b886e64, the bug went away.

However, after I made that fix on a fresh tree, the bug still happened. It turns out that input.setValueFromResult still ends up getting called, by UrlbarView._selectItem [2], which is called when results are received [3]. The fix for this afaict is just to pass `updateInput: false` to _selectItem.

The autofill-related fix doesn't seem to be necessary at all anymore (likely due to the substantial changes to autofill since that bug landed), but I left it in anyway since it seems right to not allow autofill results for the history popup.

One other useful bit of info is that userTypedValue is set to null by tabbrowser on page load [4], so that's how userTypedValue has a null value when the bug manifests and it goes from null to non-null.

[1] https://hg.mozilla.org/mozilla-central/file/5e2a3b886e647af1968b9e52a6672bdeee2a0d6f/browser/components/urlbar/UrlbarController.jsm#l150
[2] https://searchfox.org/mozilla-central/rev/da14c413ef663eb1ba246799e94a240f81c42488/browser/components/urlbar/UrlbarView.jsm#685
[3] https://searchfox.org/mozilla-central/rev/da14c413ef663eb1ba246799e94a240f81c42488/browser/components/urlbar/UrlbarView.jsm#220
[4] https://searchfox.org/mozilla-central/rev/da14c413ef663eb1ba246799e94a240f81c42488/browser/base/content/tabbrowser.js#5118

Differential Revision: https://phabricator.services.mozilla.com/D35505

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Drew Willcoxon 2019-06-21 18:37:27 +00:00
parent 33956ae957
commit ed7b16480f
3 changed files with 46 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
browser/components/urlbar/UrlbarInput.jsm
View File

@ -1325,7 +1325,7 @@ class UrlbarInput {
this.editor.selectAll();
event.preventDefault();
} else if (this.openViewOnFocus && !this.view.isOpen) {
this.startQuery();
this.startQuery({ allowAutofill: false });
}
return;
}
@ -1334,7 +1334,7 @@ class UrlbarInput {
if (this.view.isOpen) {
this.view.close();
} else {
this.startQuery();
this.startQuery({ allowAutofill: false });
}
}
}

4
browser/components/urlbar/UrlbarView.jsm
View File

@ -217,7 +217,9 @@ class UrlbarView {
});
} else {
// Clear the selection when we get a new set of results.
this._selectItem(null);
this._selectItem(null, {
updateInput: false,
});
}
// Hide the one-off search buttons if the search string is empty, or
// starts with a potential @ search alias or the search restriction

50
browser/components/urlbar/tests/browser/browser_dropmarker.js
View File

@ -4,17 +4,49 @@
"use strict";
add_task(async function() {
await BrowserTestUtils.withNewTab({ gBrowser, url: "http://example.com/" }, async () => {
await UrlbarTestUtils.promisePopupOpen(window, () => {
let historyDropMarker =
window.document.getAnonymousElementByAttribute(gURLBar.textbox, "anonid", "historydropmarker");
EventUtils.synthesizeMouseAtCenter(historyDropMarker, {}, window);
});
let queryContext = await gURLBar.lastQueryContextPromise;
add_task(async function basic() {
await BrowserTestUtils.withNewTab("http://example.com/", async () => {
let queryContext = await clickDropmarker();
is(queryContext.searchString, "",
"Clicking the history dropmarker should initiate an empty search instead of searching for the loaded URL");
is(gURLBar.value, "example.com",
is(gURLBar.value, "http://example.com/",
"Clicking the history dropmarker should not change the input value");
await UrlbarTestUtils.promisePopupClose(window,
() => EventUtils.synthesizeKey("KEY_Escape"));
});
});
add_task(async function proxyState() {
// Open two tabs.
await BrowserTestUtils.withNewTab("about:blank", async browser1 => {
await BrowserTestUtils.withNewTab("http://example.com/", async browser2 => {
// Click the dropmarker on the second tab and switch back to the first
// tab.
await clickDropmarker();
await UrlbarTestUtils.promisePopupClose(window,
() => EventUtils.synthesizeKey("KEY_Escape"));
await BrowserTestUtils.switchTab(
gBrowser,
gBrowser.getTabForBrowser(browser1)
);
// Switch back to the second tab.
await BrowserTestUtils.switchTab(
gBrowser,
gBrowser.getTabForBrowser(browser2)
);
// The proxy state should be valid.
Assert.equal(gURLBar.getAttribute("pageproxystate"), "valid");
});
});
});
async function clickDropmarker() {
await UrlbarTestUtils.promisePopupOpen(window, () => {
let historyDropMarker =
window.document.getAnonymousElementByAttribute(gURLBar.textbox, "anonid",
"historydropmarker");
EventUtils.synthesizeMouseAtCenter(historyDropMarker, {}, window);
});
let queryContext = await gURLBar.lastQueryContextPromise;
return queryContext;
}