1670932 - The search results hyperlink is missing from the megabar after a search mode

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Raluca Popovici, Desktop QA]

Attachment: 2020-10-13_11h10_16.png

Note:
There's a certain requirement that we haven't figured out which makes it mildly intermittent.

Affected Versions:
Nightly 83.0a1 (2020-10-12) (64-bit)

Tested on:
Windows 10 x64

Steps to Reproduce:

1. Launch Firefox and open a new tab
2. Browse any movie website
3. Insert a text in the megabar
4. Select a search engine from the One-click search engines
5. Press Enter key

Expected Results:
Search mode is retained replacing the search results hyperlink in the megabar

Actual Result:
The search results hyperlink is missing from the megabar after a search mode

Regression-Range:
Regressed by Bug 1655486 - Support search mode across sessions. r=mak

Severity Suggestion:
S3 severity

Comment 1

[Marco Bonardo [:mak]]

may be a P3 if it happens rarely, but it would be better to investigate it sooner than later.

Comment 2

[Pablo]

Hi marco, i was also checking this and i found this:

1. Open firefox with brand new profile: (when its open i see 2 tabs opened )
2. on first tab i click on url bar and type "argentina", then i click on google one off and hit enter key.
   Result: Search is done and i see the link in URL bar. ( no search shortcut button is shown)

but:

1. Open firefox with brand new profile: (when its open i see 2 tabs opened )
2. OPEN A brand new tab (3rd tab)
3. click on url bar and type "argentina", then click on google one off and hit enter.
   Result: the URL link is not visible anymore, and if i click on URL bar it stayed with the search mode button "google"

i can then close that tab, repeat the same steps on a brand new tab, and i see the link in the url bar.

Comment 3

[Pablo]

I can also reproduce this using Wikipedia one-off, does not matter in which tab i am positioned:

-type "sao paulo" and click on wikipedia one off and hit enter key : i can see the link in url bar.

-type "amsterdam" and click on wikipedia one off and hit enter key : the link in url bar is gone and the search mode button stayed.

Comment 4

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59af6a8368d49c8236ef97f268df8e74be4c4b39

Comment 5

[Drew Willcoxon :adw]

I can reproduce this pretty easily. It's due to the fact that a remoteness change happens when loading a SERP from about:newtab, and remoteness changes trigger a tab restore, which I didn't realize. The restore races the gURLBar.setURI call. If it happens after setURI, then the search mode is "restored" even though you navigated to a new page. So this is another manifestation of the race between restore and setURI that made bug 1655486 so difficult.

There's actually another setURI call that happens first, before the restore. So if the restore happens before the second setURI call, then the search mode flickers briefly. i.e., it goes like: new URL -> search mode -> new URL. So either way, it don't look great.

The code I modified in session store already checks for a remoteness change before trying to restore userTypedValue. If the restore is due to a remoteness change, then it doesn't restore userTypedValue. So we just need to do the same for search mode.

AFAICT this bug doesn't happen when the current tab is not about:newtab, i.e., when a remoteness change is not necessary, despite what comment 0 says about navigating to a movie site.

Comment 6

[Drew Willcoxon :adw]

Attachment: Bug 1670932 - Don't restore urlbar search mode due to remoteness change.

When navigating to a new URL that requires a remoteness change, session store
restores the tab after the change. The restore races the gURLBar.setURI call
that happens due to the location change. If the restore happens after the
setURI call, then the search mode is "restored" even though a new page was
loaded.

The session store code surrounding the chunk that I modified in bug 1655486
checks for a remoteness change before trying to restore userTypedValue. If the
restore is due to a remoteness change, then it doesn't restore userTypedValue.
We just need to do the same for search mode.

Also, I think we should be using TabStateCache here, not TabState.collect.
TabStateCache is updated in restoreTab and is available throughout the
restore process. TabState.collect on the other hand is a live view of the
given tab, so if search mode happens to be active when collect is called, then
the returned tab state will have searchMode defined, which is not what we want
here. But I'm not sure why the surrounding code here uses TabState.collect
instead of TabStateCache in order to restore userTypedValue. It seems like
an error to me, but I might be missing something.

Due to the racey nature of this bug, writing a test isn't so easy, so this patch
doesn't have one. It will be obvious through manual testing if this regresses.

Comment 7

[Drew Willcoxon :adw]

Looks like this might fix the intermittent in bug 1671045 too.

Comment 8

[Drew Willcoxon :adw]

Nope, that try run didn't run browser_searchMode_sessionStore.js in verify mode for some reason... [Edit: I didn't have my patch to bug 1659203 applied.]

Comment 9

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8a5633128a5c9cda871b47faec9fbeba4f465ff

Comment 10

[Marco Bonardo [:mak]]

(In reply to Drew Willcoxon :adw from comment #8)

Nope, that try run didn't run browser_searchMode_sessionStore.js in verify mode for some reason... [Edit: I didn't have my patch to bug 1659203 applied.]

I think only tests that have been touched by the patch run in verify. Also "This test exceeded the timeout threshold" only says the test is taking too much time and should be splitted, but the test didn't fail.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1655486

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff449df608ec
Don't restore urlbar search mode due to remoteness change. r=mak

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ff449df608ec

Comment 14

[Raluca Popovici, Desktop QA]

Reproduced the initial issue in Nightly 83.0a1 (2020-10-12) using Windows 10.
Verified - Fixed in latest Nightly 84.0a1 (2020-10-26) and Beta 83.0b4( 2020-10-25) using Windows 10 and Ubuntu 18.04.