Adaptive history logic breaks when the user 'forgets' the website and also has a bookmark of it
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
People
(Reporter: cbaica, Assigned: adw)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Affected versions
- Fx 102.0b4
- Fx 103.0a1
Affected platforms
- Windows 10
- Ubuntu 20.04
- macOS
Steps to reproduce
- Launch Firefox.
- Navigate to Reddit and click on a post from the page.
- Open a new tab and close the previously accessed page from step 2.
- Type 'redd' in the address bar and choose the reddit post from the Firefox Suggest section. (at this point the adaptive history for 'redd' is created).
- Bookmark the page.
- Open a new tab and close the previously accessed page from step 4.
- Open history (either library or sidebar) and use "Forget about this site" to remove the reddit post entry from history.
- Delete the bookmark from the bookmark toolbar.
Expected result
- No adaptive history for autofil remains (the autofill for 'redd' no longer exists).
- No records exist when checked for in the browser console.
Actual result
- Adaptive history autofill triggers briefly (flickers) for every letter of the word 'redd' that is typed in the address bar.
Regression range
- This is not a regression.
Additional notes
- This seems to be because of the weird interaction between 'Forget about this site' functionality and bookmarking.
What I mean by this is that:
- if bookmark is removed first, then the forget about this site is used, there is no adaptive history.
- if the user just DELETES the history entry and then removes the bookmark, no adaptive history entry remains.
- if an adaptive autofill entry is done on a record that exists only in history and the 'forget about this site' functionality is used on the history entry, the adaptive autofill entry is removed.
- if an adaptive autofill entry is done on a record that exists only in bookmarks and then the user deletes the bookmark, the adaptive autofill entry is removed.
Reporter | ||
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
Thanks Cristian, this looks like an autofill placeholder problem. It's a relatively minor severity because it doesn't affect functionality (correct me if I'm wrong), but it's ugly flickering we should fix soon.
Are there STR that don't require adaptive history autofill, i.e., with it disabled? I'm wondering if it's actually unrelated. Don't worry about spending too much time trying to find out.
Assignee | ||
Comment 2•2 years ago
•
|
||
I was wrong, this isn't just about the placeholder. There are two problems:
- The placeholder is incorrectly autofilled when the provider does not return an autofill result (before you type the full
input
in the record), causing flickering - Using the STR above, the provider still returns an autofill result (after you type the full
input
in the record)
The second problem is pretty bad. It happens because the moz_inputhistory
record still exists; however, in moz_places
, the frecency and visit count are both zero. For adaptive autofill, all we check is moz_inputhistory
. We need to also make sure the frecency and/or visit count are > 0, probably.
Note that this is not a problem for the input history provider. If I disable adaptive autofill and type the record's input, no input history result is shown. AFAICT that's because the input history provider uses autocomplete_match()
to filter rows, and autocomplete_match()
normally allows a row only when its visit count is > 0.
That said, the adaptive history record is eventually deleted, I'm not sure by what or how long it takes (a few minutes in my testing) but IIRC Places performs periodic cleanups.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 3•2 years ago
|
||
Problem 1 (related to the placeholder) was kind of addressed by bug 1584545, but AFAICT in order to avoid flicker we also need to set _autofillPlaceholder
to an empty string when the input receives the first result and it is not an autofill. The code added by that bug is now here.
Assignee | ||
Comment 4•2 years ago
|
||
I filed bug 1773918 for problem 1 since I ended up fixing some other problems related to the placeholder. Let's focus this bug on problem 2.
Assignee | ||
Comment 5•2 years ago
|
||
This makes the adaptive history query use the same conditions for each of the
sources that the URL query uses. This fixes the bug, and it also makes sure that
adaptive history autofill and URL autofill generally occur under the same
conditions, which is desirable IMO.
I also moved urlCondition
to the end of the WHERE clause as an optimization,
so the other parts of the WHERE clause can short-circuit its string comparison.
Assignee | ||
Comment 6•2 years ago
|
||
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7c8aa5bb8751 Make the adaptive history autofill conditions the same as the URL autofill conditions so non-visited zero-frecency URLs are not autofilled. r=daisuke
Comment 8•2 years ago
|
||
bugherder |
Comment 9•2 years ago
|
||
The patch landed in nightly and beta is affected.
:adw, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox102
towontfix
.
For more information, please visit auto_nag documentation.
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Reporter | ||
Comment 10•2 years ago
|
||
This is verified fixed in Fx103.0a1 on Windows 10. The adaptive history record is now correctly removed from firefox.
Description
•