1647896 - Add fake one-offs for history/bookmarks/etc

Status: VERIFIED FIXED

(Firefox :: Address Bar, enhancement, P3)

Description

[Harry Twyford]

These would only be visible in the Urlbar's one offs and would act the same way the history/bookmark/etc restriction tokens do today.

If the user removes all engines but their default, we should continue to hide the one-offs like we do today, even if these would otherwise still be showing. We should also remove them if the user disabled history/bookmarks/etc search in the Urlbar.

We also need icons for these one-offs from UX.

Comment 1

[Aaron Nowack]

If the user removes all engines but their default, we should continue to hide the one-offs like we do today, even if these would otherwise still be showing. We should also remove them if the user disabled history/bookmarks/etc search in the Urlbar.

Please consider instead letting the user explicitly choose whether or not to display these one-offs separately from which if any search engine one-offs are enabled, the same way individual search engines can be displayed or not today.

I can easily imagine someone liking the ability to easily/quickly filter to history or bookmarks only while not wanting or needing multiple search engines. It may also make sense for some users to not search history and/or bookmarks by default, but still be able to select the one-off to do that kind of search when desired.

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1647896 - Add local search mode one-offs for history, bookmarks, and tabs.

Comment 3

[Drew Willcoxon :adw]

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f00da8ba6160
Add local search mode one-offs for history, bookmarks, and tabs. r=harry,fluent-reviewers,flod

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for urlbar failures.

backout: https://hg.mozilla.org/integration/autoland/rev/2ba712ae046e081087c304c6a8a421f2ce6bceed

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f00da8ba616069eae4e702bc92c44b068097519e&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311445610&repo=autoland&lineNumber=2178

[task 2020-07-29T22:36:04.577Z] 22:36:04 INFO - GECKO(1262) | [Parent 1262: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 2 (0x7fd105a04130) [pid = 1262] [serial = 5] [outer = (nil)] [url = about:blank]
[task 2020-07-29T22:36:04.577Z] 22:36:04 INFO - GECKO(1262) | [Parent 1262: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 1 (0x7fd0f9e0fc00) [pid = 1262] [serial = 32] [outer = (nil)] [url = about:blank]
[task 2020-07-29T22:36:04.584Z] 22:36:04 INFO - GECKO(1262) | [Parent 1262: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 0 (0x7fd0ff9e5c00) [pid = 1262] [serial = 9] [outer = (nil)] [url = resource://gre-resources/hiddenWindow.html]
[task 2020-07-29T22:36:04.620Z] 22:36:04 INFO - GECKO(1262) | [Parent 1262, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp, line 3359
[task 2020-07-29T22:36:04.677Z] 22:36:04 INFO - GECKO(1262) | [Parent 1262, Main Thread] WARNING: NS_ENSURE_TRUE(Preferences::InitStaticMembers()) failed: file /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp, line 4343
[task 2020-07-29T22:36:04.738Z] 22:36:04 INFO - TEST-INFO | Main app process: exit 0
[task 2020-07-29T22:36:04.738Z] 22:36:04 INFO - TEST-INFO | Confirming we saw 42 DOCSHELL created and 42 destroyed log strings.
[task 2020-07-29T22:36:04.738Z] 22:36:04 INFO - TEST-INFO | Confirming we saw 115 DOMWINDOW created and 115 destroyed log strings.
[task 2020-07-29T22:36:04.739Z] 22:36:04 ERROR - TEST-UNEXPECTED-FAIL | accessible/tests/browser/events/browser_test_selection_urlbar.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/browser.xhtml]
[task 2020-07-29T22:36:04.740Z] 22:36:04 ERROR - TEST-UNEXPECTED-FAIL | accessible/tests/browser/events/browser_test_selection_urlbar.js | leaked 1 window(s) until shutdown [url = about:blank]

Comment 6

[Natalia Csoregi [:nataliaCs]]

Also failing:

• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311445687&repo=autoland&lineNumber=57460
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311445683&repo=autoland&lineNumber=9054
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311446377&repo=autoland&lineNumber=28431
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311445581&repo=autoland&lineNumber=4723
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311448706&repo=autoland&lineNumber=16073
• other browser-chrome urlbar related failures

Comment 7

[Drew Willcoxon :adw]

The window is leaking because the window closes but the prefs observer is still holding on to it via the one-offs. I added the observer since the try runs above. Need to remove the observer when the window closes and/or hold it weakly.

Comment 8

[Drew Willcoxon :adw]

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

Comment 9

[Marco Bonardo [:mak]]

The pref cannot change while the view is open (or is quite unlikely), why do we need to observe the pref change rather than just check when we open the view and build one-offs?

Comment 10

[Drew Willcoxon :adw]

The one-offs rebuild themselves only when necessary. If the engines haven't changed and the view/panel width hasn't changed, __rebuild bails. So the pref observer nils out _engines when a local-related pref changes so that __rebuild doesn't bail.

Comment 11

[Marco Bonardo [:mak]]

That's all under our control though, we could check the prefs before bailing out and rebuild if state changed.

Comment 12

[Drew Willcoxon :adw]

So we'd need to remember the old state of the prefs to be able to compare to the new state... Why not just have a pref observer? Why is that a problem?

Comment 13

[Marco Bonardo [:mak]]

I'm not saying it's a problem by itself, I'm suggesting an alternative to avoid having to bookkeep the listeners if that becomes a problem.

Comment 14

[Drew Willcoxon :adw]

It's not a problem. I just factored out the existing observer into an exported class. It's nice actually.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d51ee7e1839
Add local search mode one-offs for history, bookmarks, and tabs. r=harry,fluent-reviewers,flod

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2d51ee7e1839

Comment 17

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I verified this issue using 82.0a1 (2020-09-14) on macOS 10.13 and Windows 10 x64.
Adrian could you help me with Ubuntu verification?

Comment 18

[Petruta Horea [:phorea], PTO until 8/22/25]

Verified as fixed on Ubuntu 18.04 using Firefox 81RC and latest Nightly 83.0a1 builds. Due to bug 1662477, in latest Nightly, local one-off are still shown even if urlbar privacy preferences are disabled.