1647899 - Retain or remove search mode across tabs

Status: RESOLVED FIXED

(Firefox :: Address Bar, enhancement, P2)

Description

[Harry Twyford]

This is related to our retained results behaviour. We should be aware of edge cases around engine removal: say the user enters search mode with a particular engine, switches tabs, removes that engine, then returns to the initial tab.

Comment 1

[Drew Willcoxon :adw]

There's the question of session store too... We keep userTypedValue in session store, so if you type something in the urlbar and restart Firefox for example, the value you typed is restored. I imagine we want to treat search mode just like that.

I'm not sure yet whether that fits into this bug or not. It's different enough conceptually for it to be its own bug, but if the fix for this bug ends up creating a userSearchMode, which it might, then it may be more straightforward to do that here.

Comment 2

[Drew Willcoxon :adw]

Ah, that's bug 1655486. And the more I think about this, I think it would be a good idea to fix this bug as narrowly as possible since we want to land this by Monday. Like, just a map in UrlbarInput that stores search modes for browsers -- something that works well enough for initially enabling in Nightly, and then we can refine or even replace it under the hood later.

Comment 3

[Drew Willcoxon :adw]

(In reply to Harry Twyford [:harry] from comment #0)

... removes that engine...

I filed bug 1657801 for this since it's orthogonal to tabs (this bug).

Comment 4

[Drew Willcoxon :adw]

Attachment: Bug 1647899 - Retain or remove search mode across tabs.

This is very simple, but it gets the job done. It keeps a map in UrlbarInput
from browsers to search mode objects. The map is updated in setSearchMode, and
it's used to set the search mode in _afterTabSelectAndFocusChange.

Comment 5

[Drew Willcoxon :adw]

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

Comment 6

[Marco Bonardo [:mak]]

fwiw, long term I'd prefer us storing something in the tab's browser, similar to the typed string property. Maybe it could be a good time to introduce a browser.urlbarState object.

Comment 7

[Drew Willcoxon :adw]

Yes, that's my thought too. I don't think we can get that done by Monday and that's why I chose this simple approach.

Actually, long term it would be nice to move that state out of browser and into urlbar. It's urlbar state, it should be in urlbar. When it's just a string, userTypedValue, it's not a big deal, but we're now adding more state and who knows what else in the future.

Comment 8

[Drew Willcoxon :adw]

I mean that's basically what I said in comment 1.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25280cb70b0f
Retain or remove search mode across tabs. r=harry

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/25280cb70b0f

Comment 11

[Marco Bonardo [:mak]]

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

Actually, long term it would be nice to move that state out of browser and into urlbar. It's urlbar state, it should be in urlbar. When it's just a string, userTypedValue, it's not a big deal, but we're now adding more state and who knows what else in the future.

I think the main reason for it being in browsers is SessionRestore

Comment 12

[Drew Willcoxon :adw]

Yes, but I'd like to look into the possibility of hooking up urlbar to sessionstore.

Comment 13

[Pablo]

I verified this issue using 82.0a1
on macOS 10.13 and Windows 10 x64 and ubuntu 20.04