Closed Bug 1493636 Opened 6 years ago Closed 6 years ago

Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold

Categories

(Firefox :: Address Bar, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 64
Tracking Status
firefox62 --- unaffected
firefox63 --- verified
firefox64 --- verified

People

(Reporter: danibodea, Assigned: adw)

References

Details

(Whiteboard: [fxsearch])

Attachments

(2 files)

[Note]: - This is a followup on bug 1488879, based on comment https://bugzilla.mozilla.org/show_bug.cgi?id=1488879#c36, using the STR from comment https://bugzilla.mozilla.org/show_bug.cgi?id=1488879#c49 and considering the user's dissatisfaction from bug's comments, the process of the auto-filling of the address bar with previously saved bookmarks is not working as expected. It appears that a sub-groups of users would like bookmarks to get auto-filled by default, indifferent to the frequency of the bookmark's visitation (as explained in comment https://bugzilla.mozilla.org/show_bug.cgi?id=1488879#c35). [Affected versions]: - 64.0a1, 63.0b9, 62.0.2 [Affected platforms]: -All [Steps to reproduce]: 1. Clear browser history and uncheck "remember my history" 2. Save two bookmark of different domains, https://www.mozilla.org/ and https://www.youtube.com/ (autofill should work properly for now) 3. Save another bookmark to https://www.youtube.com/user/Mozilla 4. Type "moz" in the address bar. The URL should be auto-filled as mozilla.org, but it does not. 5. Type "you" in the address bar. [Expected result]: - The URL should is auto-filled as youtube.com; any of the bookmarks should get auto-filled. [Actual result]: - It turns out If I have two or more bookmarks of the same domain, other bookmarks of unique domains will not be auto-filled.
Severity: normal → enhancement
See Also: → 1488879
Unfortunately if history is disabled we don't have the last time a bookmark was accessed, the only data we could rely on is the decayed frecency, though it may not be that good (because maybe I created a bookmark 10 years ago, but I use it everyday). It was working before because we were not clearing the typed information with history, that is also wrong. We could distinguish 2 cases: 1. history is enabed: use the normal threshold 2. history is disabled, or clearonshutdown is enabled: autofill bookmarks The problem is in the middle, what if history is enabled but the user clears history completely? From that point on, nothing gets autofilled anymore. Maybe in the end the simplest path is what you suggested, just always autofill bookmarks. Though, that requires a performant way to figure out if an origin is bookmarked.
Flags: needinfo?(adw)
Priority: -- → P1
Whiteboard: [fxsearch]
I don't get it: When the "browsing and download history" checkbox is unchecked, the browsing and download history is NOT supposed to get deleted. That's the reason for this checkbox, as far as I apprehend.
Axel, I'm sorry, reopening your report and keeping it separate, it looks like I misread your log.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Flags: needinfo?(adw)
I still think exempting bookmarks from the autofill frecency threshold is probably the right thing to do, but I'd also like to test/investigate why exactly they aren't getting autofilled in these cases -- and there are slightly different cases being reported. As long as your history is 100% cleared, then the threshold should be zero, so bookmarks *should* be autofilled since we fixed bug 1488879. Once you start visiting URLs after that, if you are not in permanent private browsing mode, then the threshold will start to go up, and in that case it's clear why unvisited bookmarks would not autofill. Exempting bookmarks is the fix for that. But, if you are in permanent private browsing mode, theoretically autofilling bookmarks should Just Work, because presumably frecencies never change, so the threshold remains at zero. I need to look into how that works.
(In reply to Drew Willcoxon :adw from comment #5) > I still think exempting bookmarks from the autofill frecency threshold is > probably the right thing to do, but I'd also like to test/investigate why > exactly they aren't getting autofilled in these cases -- and there are > slightly different cases being reported. As long as your history is 100% > cleared, then the threshold should be zero I'm not sure about this. https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/toolkit/components/places/SQLFunctions.cpp#832 a bookmark, even if unvisited, won't have a zero frecency, and as such I'd expect the threshold to not be zero.
Ah yes, you're right
Comment on attachment 9014235 [details] Bug 1493636 - Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold. Marco, this just unconditionally includes the bookmarked subquery in the main query, and then does `HAVING host_frecency >= ${SQL_AUTOFILL_FRECENCY_THRESHOLD} OR bookmarked`. This seems like a simple fix that we can uplift to address the problem immediately, as opposed to a more comprehensive fix that we've briefly discussed that possibly involves a schema change. IMO we could take this now but still continue to evaluate a more performant fix involving a schema change. However, we do have an index on moz_places.origin_id, so maybe there's not much of a perf win to be had here? And obviously we already use this exact query when sugest.history is disabled, but we'd be extending that perf to all users with this, not only those with suggest.history=false. What do you think? I know you already had some comments on this in a similar patch in bug 1489060 comment 7, but here I'm not doing the extra sorting.
Attachment #9014235 - Flags: feedback?(mak77)
Comment on attachment 9014235 [details] Bug 1493636 - Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold. off-hand it sounds ok to me, there is a small perf hit but it looks acceptable
Attachment #9014235 - Flags: feedback?(mak77) → feedback+
Thanks. This bitrots bug 1494471, but I'll handle that one after this one.
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e9c7d14fc4a9 Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold. r=mak
Flags: needinfo?(adw)
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/237c55ddb112 Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold. r=mak
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Please see comment 0 for one set of STR. Additional STR, similar to comment 0 but not the same: 1. Save two bookmarks of different domains, https://www.mozilla.org/ and https://www.youtube.com/ 2. Save another bookmark, https://www.youtube.com/user/Mozilla 3. Clear history (using "last hour" or "everything", shouldn't matter) 4. Type "moz" in the address bar. The URL should be auto-filled as mozilla.org. 5. Type "you" in the address bar. The URL should be auto-filled as youtube.com.
Flags: qe-verify+
Attached patch Beta/63 patchSplinter Review
[Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1239708 User impact if declined: Bug 1239708 (new autofill algorithm) landed in 62, and this bug landed in 64. So if it's not uplifted to 63, users will have to wait another cycle before they see this bug fixed. 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 17 List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): The patch makes slight tweaks to two SQL queries used in autofill. This code has automated tests, and I've verified manually. Most of this patch is test additions/changes. String changes made/needed: None
Attachment #9015100 - Flags: approval-mozilla-beta?
Summary: Improve the auto-fill process of the address bar, considering bookmarked pages → Always autofill bookmarks, regardless of their frecency and the autofill frecency threshold
I've just installed Nightly to test it out and I can confirm that autofill works once again, great! Looking forward to the backport to 63 beta.
I've successfully reproduced this bug on Firefox 63.0a9 and I can Confirm Autofill is working back again on Firefox 64.0a1.
I have tested as well, it is working back again in Firefox 64.0a1. Thank you!
Comment on attachment 9015100 [details] [diff] [review] Beta/63 patch Follow up fix to bug 1488879 which was a tracked regression for 63 that already got uplifted, fix verified on Nightly, uplift approved for 63 beta 13, thanks.
Attachment #9015100 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
The STR from comment 0 and comment 17 were attempted and the results are promising: This issue is now fixed in Nightly v64.0a1 from 2018-10-08. Awaiting Beta build for uplift verification. Thank you!
Status: RESOLVED → VERIFIED
I've been testing it out properly with beta 13 and I'm sad to report that it's not completely fixed yet. The autofill works the first time when you haven't visited any pages on the bookmarked page yet. Once you visit some pages on the bookmarked page (tabs or no tabs doesn't seem to matter) and you then type in the name of the bookmarked page again it's no longer autofilling anything.
The test cases in comment 0 and comment 17 (with both history enabled and not) are passed (as expected) in Firefox Beta v63.0b13, on my test machine with Windows 10 x64. @maarnuwels: I could not reproduce your case and I've tried this STR: 1. Clear browser history and set browser not to "remember my history"; 2. Save two bookmarks of different domains, https://www.mozilla.org/ and https://www.youtube.com/; 3. Save another bookmark, https://www.youtube.com/user/Mozillam; 4. Navigate to several different child pages of the https://www.mozilla.org/ and https://www.youtube.com/ domains; 5. Type "moz" in the address bar. -> The URL is auto-filled as mozilla.org as expected. 6. Type "you" in the address bar. -> The URL is auto-filled as youtube.com as expected. Are my steps different from yours? If yes, please write detailed steps to reproduce with actual and expected results that will show the issue you are experiencing. Thank you for your contribution!
Flags: needinfo?(maarnuwels)
https://bugzilla.mozilla.org/show_bug.cgi?id=1493636#c26 I have many bookmarks set. As an example, I'm using http://tweakers.net (which I also have as a homepage). After manually clearing history, if I enter tweak, it autofills OK. I then visit the homepage, i.e., just open the bookmark in same tab. Then I try to autofill tweakers again (with 'tweak' again), and tweakers.net isn't autofilled anymore. Actually, while I'm writing this, I think I found the problem. Most of my bookmarks are still http:// (been a Firefox user since 1.0!), but most sites nowadays use https:// of course and redirect to it. It seems that all bookmarks set to http:// will break autofill after the page has been loaded once. If the bookmark is https://, it seems to work OK. I think this issue needs to be addressed. Not on my end as it used to work fine in previous Firefox version (+ migrating all bookmarks to https:// is no option for me, I have about 2500).
Flags: needinfo?(maarnuwels)
*visit the homepage -> visit the filled bookmark. I can't find out how to edit my previous comment.
Sorry, I can't reproduce that. Once I bookmark http://tweakers.net, tweakers.net always autofills. We do handle both http and https.
I have tested your presumption that your issue could be related to having bookmarks saved with http instead of https but I still can't reproduce it. Considering that you are performing the tests in your old profile, it may be caused by add-ons or extensions. Please test if the issue is reproducible in safe mode, here is a link that can help you: https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode If you still have the issue please create a new profile, you have the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager and retest the issue. These tests are necessary to know whether the issue is linked to extensions or profile related settings. I will have to mark this issue as verified, but let's continue talking on this thread until we will hopefully find the cause. Thank you!
Flags: qe-verify+ → needinfo?(maarnuwels)
If it would work in safe mode; how would I then determine what is causing the issue?
Flags: needinfo?(maarnuwels)
I just disabled places.sqlite and so far so good. Shouldn't that file be reset if you clear history on close?
It's no longer working again, sadly. I'll investigate this in safe mode too.
Safe mode isn't working either for me, I'll now see if a new profile is working.
Depends on: 1502666
Depends on: 1503293
Depends on: 1502821
No longer depends on: 1503293
Regressions: 1503293
Regressions: 1560693
No longer regressions: 1560693
No longer depends on: 1502821
Regressions: 1502821
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: