Closed Bug 1602728 Opened 4 months ago Closed 3 months ago

Unvisited bookmarks do not autocomplete with browser.urlbar.suggest.history = false

Categories

(Firefox :: Address Bar, defect, P1)

69 Branch
defect
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 74
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- verified
firefox74 --- verified

People

(Reporter: ke5trel, Assigned: adw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

STR:

  1. Set browser.urlbar.suggest.history = false.
  2. Clear all history.
  3. Type "blog" in the address bar.

Expected:

Autocompletes to blog.nightly.mozilla.org bookmark.

Actual:

Bookmark does not autocomplete despite browser.urlbar.suggest.bookmark being enabled.

Visit the bookmark or enable browser.urlbar.suggest.history and autocomplete works as expected.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e9c9a624d055b54cf65a5361f5261a2278ec6d3e&tochange=36e230321fd00eab502391f7a16aff4aef35364d

Regressed by Bug 1582234.

sigh, so many conflicting options.

Priority: -- → P1
Points: --- → 3

The test includes all combinations of suggest.history and suggest.bookmark. I wonder what's going wrong.

Getting late for a fix in 72 though if you land a fix in 73 and verify it, we could still consider uplift.

Assignee: nobody → adw
Status: NEW → ASSIGNED

With suggest.history && suggest.bookmark, we do:

HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
       OR bookmarked

So bookmarks bypass the frecency threshold.

With !suggest.history && suggest.bookmark, we do:

HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
       AND bookmarked

So bookmarks are subject to the frecency threshold, which is inconsistent with the previous case.

Before bug 1582234, we did this for !suggest.history && suggest.bookmark:

WHERE bookmarked
HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD}
       OR bookmarked

So this is indeed a regression from bug 1582234, but it doesn't show up until a bookmark falls below the frecency threshold, and maybe that's why the test doesn't catch it. (Or of course maybe the test is wrong too.)

Duplicate of this bug: 1605942
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40bdfc030ef8
Always autofill bookmarks when suggest.history=false and suggest.bookmark=true regardless of frecency threshold. r=mak
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74
Flags: qe-verify+
Flags: in-testsuite+

Comment on attachment 9119934 [details]
Bug 1602728 - Always autofill bookmarks when suggest.history=false and suggest.bookmark=true regardless of frecency threshold.

Beta/Release Uplift Approval Request

  • User impact if declined: Unvisited bookmarks sometimes will not be autofilled.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Please see 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): This is a small tweak to the autofill SQL code, so it doesn't affect anything else. Autofill is very well tested in our automated tests.
  • String changes made/needed:
Attachment #9119934 - Flags: approval-mozilla-beta?

Comment on attachment 9119934 [details]
Bug 1602728 - Always autofill bookmarks when suggest.history=false and suggest.bookmark=true regardless of frecency threshold.

Fixes an autofill regression. Thanks for including new tests. Approved for 73.0b5.

Attachment #9119934 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I reproduced this issue using Fx 73.0a1 (2019-12-10) on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 74.0a1(latest) and Fx 73.0b5, on Windows 10 x64, Ubuntu 18.04 LTS and macOS 10.15.

Status: RESOLVED → VERIFIED
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: qe-verify+
Duplicate of this bug: 1612691
You need to log in before you can comment on or make changes to this bug.