1633030 - "Search with Google" sometimes gets prepended to URLs in autocomplete suggestions (i.e. history matches are displayed as if they were searches)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Daniel Holbert [:dholbert]]

Attachment: image.png

I don't have reliable STR for this, but I managed to get a screenshot (see attached).

The bug: sometimes when I'm typing a search query into my address bar, and I also have history matches for it, and I hit "downarrow", then:

• "Search with Google" label suddenly gets at the start of the URL for my history matches
• The icon for these history matches changes to a magnifying glass, as if they were a suggested search rather than a match in my history.

This seems to happen if I hit downarrow precisely at the same time as I type a character in my search term. E.g. in the attached screenshot, I was in the process of typing "fpn proxy" and I pressed downarrow and "x" simultaneously which resulted in the broken rendering that I captured here.

Comment 1

[Daniel Holbert [:dholbert]]

I first noticed this with today's Nightly, version 77.0a1 (2020-04-24), BTW (though I'd bet it's been around for longer than that).

I haven't been able to reproduce in a fresh profile, BTW. Also notable: I've **un-**checked the Show search suggestions ahead of browsing history in address bar results option in about:preferences, which is why these top few entries are history entries (albeit horked ones).

(I did try unchecking that box in a fresh profile, and still wasn't able to reproduce there. It's a tricky race-conditiony bug, so it might take a sufficient number of history entries to repro, or similar. Let me know if there's anything I can do to help diagnose, assuming I can continue to repro in my main browsing profile)

Comment 2

[Marco Bonardo [:mak]]

Uh, this is fancy.
I wonder if it is related to the recent work Harry did to reverse parse searches, there seems to be a subtle relation with the screenshot.

We should check the recently landed changes for hints at what may cause this

Comment 3

[Marco Bonardo [:mak]]

Daniel, could you please check the value of browser.urlbar.restyleSearches?

Comment 4

[Daniel Holbert [:dholbert]]

browser.urlbar.restyleSearches is false.

Comment 5

[Harry Twyford]

I'm also able to reproduce, but only under exactly the same conditions described in comments 0 and 1:

1. Show search suggestions ahead of browsing history in address bar results is unchecked
2. I'm on my normal profile, not a fresh one. Some history is needed for this.
3. I also got this to work when pressing the x in fpn proxy and then immediately pressing the down arrow, but only intermittently.

Afacit, this reproduces when the next character typed will change the set of history results and will not remove the search suggestions. On my machine, fpn prox returns different history results than fpn pro, which is why I can reproduce with the x key. I can't reproduce this when typing the o in fpn proxy because fpn pro and fpn pr return the same history results.

Even that STR is intermittent. I find the longer the profile's been running, the harder it is to reproduce.

I ran mozregression for this. I was able to reliably reproduce this bug when typing the x key in fpn proxy because it's fairly easily to reproduce on the first few searches done in a session. It narrowed it down to May 2019! I think we just haven't found it until now because of the very specific and intermittent STR.

mozregression: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=175f567cd163eb70fd015e585bb0ae74f60c2cfe&tochange=d6756af108596e5a27548d397f1dd72d44c45854

Of those two patches, bug 1535656 looks the most promising, although maybe it's possible bug 1548817 caused it as well. I'm also going to downgrade this from P1 seeing as it's been in the product for almost a year and hasn't been found until now.

Marco, can you reproduce this bug? Any thoughts?

Comment 6

[Marco Bonardo [:mak]]

It may be result of reusing DOM... I can check in the next days.

Comment 7

[Marco Bonardo [:mak]]

Just some spoken thoughts here.
Those rows are referring to the previous search, they are for "fpn pro" and not for "fpn prox".
When downarrow is pressed we "freeze" the results:
https://searchfox.org/mozilla-central/rev/3262e013550a0db7c1840a78a3878a929801fe40/browser/components/urlbar/UrlbarView.jsm#287
At this point we should be receiving no more new results from the backend.

On canceling a query we receive onQueryCancelled, that clears the stale rows timer, then we receive onQueryFinished that, if the query was not canceled, removes stale rows. Since the query was canceled stale rows are not removed and those are stale rows.
I think this was done on purpose, the user saw something, changed the selection, we should not be removing/moving results.
On the other side, this breaks user's expectation, when they are used to type "fpn prox" and see that the second result is a certain one, the next time they may press down immediately after typing it, and expect to be on that same result, but if we freeze results we may be on a result for "fpn pro".
This is a choice we must make, I'm not sure what's the best strategy off-hand, maybe the EventBufferer should wait for the second result if suggestions come after history entries?

This doesn't yet explain why those "stale" results have a bogus type, for which I must investigate further.

Comment 8

[Marco Bonardo [:mak]]

the problem is very likely in _on_SelectedOneOffButtonChanged()... the down may be actually selecting the one-off for a brief moment, and _on_SelectedOneOffButtonChanged makes assumptions about items and results, in particular it assumes there's a 1:1 relationship between them, but that's not true.

Comment 9

[Marco Bonardo [:mak]]

Attachment: Bug 1633030 - "Search with Google" sometimes gets wrongly prepended to address bar results. r=harry

Comment 10

[Marco Bonardo [:mak]]

This is very time and profile sensitive, as such it's not easy to QA. We'll verify it on our profiles, since at least three of us can reproduce.

Comment 11

[Harry Twyford]

Marco's patch fixes the problem on my profile.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d7206515219
"Search with Google" sometimes gets wrongly prepended to address bar results. r=harry

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9d7206515219

Comment 14

[Marco Bonardo [:mak]]

Daniel, could you please verify this fix in one of the next nightlies? We got Harry's verify, it would be nice to confirm with you too.

Comment 15

[Daniel Holbert [:dholbert]]

As I recall, the machine that I noticed this on (& reported the bug from) was my office desktop, which I'm unable to access for the foreseeable future due to COVID-19 restrictions. I don't recall if I ever encountered this on my laptop (the machine I'm currently working off of), so I don't know how much weight I'd put my current ability to verify the fix on that machine.

But, for what it's worth: I can't reproduce this in current Nightly on my laptop (using my main browsing profile there).

Comment 16

[Marco Bonardo [:mak]]

Well, I'll assume me and Harry, plus some testing by Daniel, is fine to verify this.