Closed Bug 1559264 Opened 2 months ago Closed 2 months ago

Address bar switches to "search mode" after opening dropdown and switching tabs.

Categories

(Firefox :: Address Bar, defect, P1)

defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 69
Iteration:
69.3 - Jun 10 - 23
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- unaffected
firefox68 --- verified
firefox69 --- verified

People

(Reporter: rrosario, Assigned: adw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached video urlbar.mov

See attached video.

STR:
1- Go to any website
2- Click on the down arrow to open the address bar dropdown
3- Switch to another tab and then switch back
4- The address bar now looks like a search bar (with magnifying glass) and the dropdown has different content than before

Priority: -- → P1

Seems to be a regression. I narrowed it down to between the 2019-03-20-11-29-39 Nightly [1] (no bug) and the 2019-03-21-10-41-32 Nightly [2] (bug). There's only quantumbar-related non-test change, bug 1529931. Tentatively marking that as the regressor, but I don't immediately see how.

Regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=25398e555020fef80c7b2a06a0d4c667e861cd6f&tochange=b2f1edb41241d3da6ded54edf38cca1d2d08325b

[1] http://ftp.mozilla.org/pub/firefox/nightly/2019/03/2019-03-20-11-29-39-mozilla-central/
[2] http://ftp.mozilla.org/pub/firefox/nightly/2019/03/2019-03-21-10-41-32-mozilla-central/

Keywords: regression
Regressed by: 1529931

On switching back to the original tab, gBrowser.userTypedValue is non-null in URLBarSetURI, so valid ends up remaining false there.

Assignee: nobody → adw
Status: NEW → ASSIGNED

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

Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4e6f4eaf5750
Quantumbar: Don't call setValueFromResult when opening the history popup. r=dao
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
Iteration: --- → 69.3 - Jun 10 - 23
Points: --- → 2
Flags: qe-verify+
Flags: in-testsuite+
Attached patch Beta patchSplinter Review

Beta/Release Uplift Approval Request

  • User impact if declined: Quantumbar debuts in 68, so it would be nice to uplift this to fix the problem described in this bug.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Please see comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's a small patch with two changes: (1) simply don't allow autofill when opening the history popup, so this change only affects when users click the dropdown arrow in the urlbar, (2) don't update the urlbar input text in the same case (i.e., when the user clicks the dropdown arrow in the urlbar).
  • String changes made/needed: None
Attachment #9073416 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]
Comment on attachment 9073416 [details] [diff] [review]
Beta patch

Perhaps a regression since 65 or 66, has automated test coverage, 2 weeks to launch, leaning towards taking it, Beta68+
Attachment #9073416 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1561057

I’ve managed to reproduce this issue on an affected Firefox 68.0b10 using Windows 10x64 by following the STR from comment 0.
This is verified fixed on Firefox 68.0b13 and Firefox 69.0a1 (2019-06-25) on the following OSes: Windows 10x64, Windows 7x64, macOS 10.14, Ubuntu 18.04x64.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.