1725652 - Show most relevant bookmarks in address bar, even when history is cleared

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[nicoenkarin]

Attachment: screenshot_dropdown_menu

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

1. Clear all history, except cookies.
2. Updated to Firefox 91.
3. Enter one letter in the urlbar, e.g. 'v'.

Actual results:

Drop-down menu showed random bookmarks with the letter 'v' in it.

Expected results:

It should show "JanVissersWeer.nl" (weather forecast), like it did before the update (see screenshot).

I have set urlbar suggestions to only show bookmarks.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Address Bar' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[nicoenkarin]

For what it's worth:
Relevant reddit thread.

Comment 3

[Harry Twyford]

Thanks for the report. If there's no history, Firefox doesn't know how often you visit a site. It would have no way of knowing which bookmarks are frequently visited, so it would rank bookmarks by creation date. Did this used to work differently in a previous version?

Comment 4

[Harry Twyford]

What's wrong here is that I recently refactored the part of the code that handles "adaptive history". Adaptive history keeps track of which strings are typed in the address bar and which results are picked when those strings are typed. The next time that string is typed, we show that picked result as the first result. In my refactor, I made it so adaptive history is only shown if history results are enabled. That was a mistake -- it should be enabled if any of history, bookmarks, or open pages are enabled, per this review comment that I missed.

[Tracking Requested - why for this release]:
Annoying regression in the address bar ranking code. This issue has existed since 91, but isn't common enough for a dot release. However, it should be fixed ASAP.

Comment 5

[nicoenkarin]

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

Thanks for the report. If there's no history, Firefox doesn't know how often you visit a site. It would have no way of knowing which bookmarks are frequently visited, so it would rank bookmarks by creation date. Did this used to work differently in a previous version?

Yes, in previous versions it worked different: showed frequently visited at the top, even when history had been deleted before.

Comment 6

[nicoenkarin]

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

What's wrong here is that I recently refactored the part of the code that handles "adaptive history". Adaptive history keeps track of which strings are typed in the address bar and which results are picked when those strings are typed. The next time that string is typed, we show that picked result as the first result. In my refactor, I made it so adaptive history is only shown if history results are enabled. That was a mistake -- it should be enabled if any of history, bookmarks, or open pages are enabled, per this review comment that I missed.

[Tracking Requested - why for this release]:
Annoying regression in the address bar ranking code. This issue has existed since 91, but isn't common enough for a dot release. However, it should be fixed ASAP.

Thanks for looking in to this issue! 🎉
Hope it will be fixed in a future release.

Comment 7

[Harry Twyford]

[Tracking Requested - why for this release]:
This is an annoying bug that is hard to detect, so things just seem subtly wrong. I don't think we should sit with that for a year in ESR. The fix is low-risk and easy to test, so I think it's appropriate for an ESR dot release.

Comment 8

[Harry Twyford]

Attachment: Bug 1725652 - Show adaptive history results if any of history, bookmarks, or open pages are enabled. r?adw!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5261e225c8e
Show adaptive history results if any of history, bookmarks, or open pages are enabled. r=adw

Comment 10

[Bruce]

Possible duplicates: bug 1725892, bug 1725723 and bug 1718989.

Comment 11

[Dorel Luca [:dluca]]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta and ESR91 approval when you get a chance.

Comment 15

[Harry Twyford]

Comment on attachment 9236213 [details]
Bug 1725652 - Show adaptive history results if any of history, bookmarks, or open pages are enabled. r?adw!

Beta/Release Uplift Approval Request

• User impact if declined: Address bar results will be subtly "wrong" for users with history results disabled. It will be more difficult for those users to access their frequently-visited sites.

This issue is especially bad for users who use permanent private browsing mode or regularly clear their history, as the broken feature is the only way for them to access frequently-used results in the address bar.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See bug 1725892 comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small, well-understood front-end change. Thorough new tests were added in this patch.
• String changes made/needed:

Comment 16

[Harry Twyford]

Comment on attachment 9236213 [details]
Bug 1725652 - Show adaptive history results if any of history, bookmarks, or open pages are enabled. r?adw!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Address bar results will be subtly wrong for users who disabled history results. While this might be tolerable for one four-week release cycle, it would be enough to drive a user away from Firefox if they had to deal with it for a year.

The issue is also worse for users in permanent private browsing mode (see beta uplift request). There may be more of those users in our ESR population (libraries, schools, etc).

• User impact if declined: See beta uplift request.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See beta uplift request.
• String or UUID changes made by this patch:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9236213 [details]
Bug 1725652 - Show adaptive history results if any of history, bookmarks, or open pages are enabled. r?adw!

Approved for 92.0b5.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b2afd8359c2

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9236213 [details]
Bug 1725652 - Show adaptive history results if any of history, bookmarks, or open pages are enabled. r?adw!

Approved for 91.1esr.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/f07e6abd3bbc

Comment 21

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

I reproduced this issue following the information in comment 15.
I verified this issue is no longer reproducible using Fx 92.0b5 and Fx 93.0a1 on Windows 10 x64, Ubuntu 20.04 and macOS 10.13.

Comment 23

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

Verified using Fx 91.1.0esr under the same environment as per comment 21.