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

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

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.

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.

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

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.

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.

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

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.]

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

(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.

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

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

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.