User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0 Build ID: 20180830143136 Steps to reproduce: 1) Have firefox to only search my bookmarks 2) Make 'www.reddit.com' and 'www.amazon.com' a bookmark 3) Set 'browser.urlbar.matchBehavior' to a value of 3 4) Type 'r' or 'a' in the urlbar (awesomebar?) Actual results: Nothing shows up. Expected results: Before Firefox 62, typing 'r' alone will show up "www.reddit.com" from my bookmark in the url bar, typing 'a' will show up "www.amazon.com", and so on. But after Firefox 62, only by typing 'www.r' will "www.reddit.com" then show up, and typing "www.a" will 'www.amazon.com' then show up.

Thanks for filing this. I don't think this has anything to do with matchBehavior. Rather, there's a problem with the SQL query we use when we search only bookmarked places. bookmarkedFragment is here: https://dxr.mozilla.org/mozilla-central/rev/703546ab6d0cb643028a1ab4fda997b38f38a2e6/toolkit/components/places/UnifiedComplete.js#332 It's used here: https://dxr.mozilla.org/mozilla-central/rev/703546ab6d0cb643028a1ab4fda997b38f38a2e6/toolkit/components/places/UnifiedComplete.js#302 The problem (I think) is that that SELECT is aggregate due to both the TOTAL(frecency) column and the GROUP BY. But bookmarkedFragment is not aggregate. So it ends up looking only at whatever moz_place ends up first in its result set. If that moz_place's foreign_count is zero, then the whole bookmarkedFragment is zero, even if there are moz_places in the result set that are bookmarked. I confirmed this by running some SQL in sqlite3. When I change bookmarkedFragment to use TOTAL, things seem to work: `(SELECT TOTAL(foreign_count) > 0 FROM moz_places WHERE moz_places.origin_id = moz_origins.id)`

And the reason this used to work (on 61 and earlier) is that the query was not an aggregate query: https://hg.mozilla.org/mozilla-central/annotate/c58f0b4dd849/toolkit/components/places/UnifiedComplete.js#l266 So it just found all the matching moz_places, ordered them, and took the top one.

Actually, looks like I broke this in bug 1469651, not the original autofill bug. I'll update the blocking status accordingly. The changeset is: https://hg.mozilla.org/mozilla-central/rev/cb9a1ff4a249 Also updating the bug summary. STR for this bug are to set browser.urlbar.suggest.bookmark = true (the default) and browser.urlbar.suggest.history = false (not the default).

[Tracking Requested - why for this release]: Unsure if we need to track this for a dot release, but putting it on the radar.

Marco, could you give f? on the approach in the WIP revision please? (There's no Phabrictor f?, right?)

I don't think exempting bookmarks from the threshold is a good idea, as many other users I have bookmarks I created years ago and never cleaned up, we don't even have something like a cleanup tool, we'd end up filling things the user really doesn't care about anymore. I'd be ok to do that only if history is disabled (permanent pb), then we are surely lacking data for the feature to work as expected, and we could enlarge the dataset only for that specific case. "First sorts by the total frecency for all origins with a given host, and then sorts by frecency of individual origins. This is maybe how this should have worked from the beginning. This also breaks tests which is why this patch is a WIP right now." This sounds expensive (sorting on a grouped column, that clearly doesn't have any kind of index), and the patch includes too many fixes (3 issues) to be clear about what it is trying to achieve. Is this change necessary to fix this bug or is it a nice-to-have? What are the alternatives? (like, could we have a bookmarked field in moz_origins, or use a better subselect in the external query for bookmarked?)

OK. My thinking about exempting bookmarks is that a bookmark is a clear expression by the user that they're interested in a URL, regardless of its current frecency. The point of the threshold is to exclude URLs that the user probably is not interested in. Bookmarks don't fit that description IMO. (In reply to Marco Bonardo [::mak] from comment #7) > be clear about what it is trying to achieve. Is this change necessary to fix > this bug or is it a nice-to-have? What are the alternatives? It's just something I realized while working on this patch, since all these questions involve the same query. I'll pare the patch down to something that just fixes this bug.

(In reply to Drew Willcoxon :adw from comment #8) > OK. My thinking about exempting bookmarks is that a bookmark is a clear > expression by the user that they're interested in a URL, regardless of its > current frecency. The point of the threshold is to exclude URLs that the > user probably is not interested in. Bookmarks don't fit that description > IMO. The point of the threshold is also to exclude urls that are no more relevant when user browsing habits change, if we'd consider relevant a bookmark created 5 years ago, we could just remove the threshold completely and go back to the previous situation where things just got stuck there forever. I'm not saying the status-quo is perfect, but I also don't like the idea of going the total opposite direction.

But not autofilling anything when there's just one suggestion/bookmark available is not a good design choice. I have search suggestions disabled and clear history on shutdown, it's completely broken.

There are various problems here, and what I suggested in comment 9 is probably not perfect, because when history is missing, any useful information about bookmarks is lost as well. I suspect in the end what Drew suggested, to autofill bookmarks regardless, is the only simple path forward. Though, I'm not sure currently we can do that in a performant way.

In the origin query, the SELECT is aggregate due to both the TOTAL(frecency) column and the GROUP BY, but bookmarkedFragment is not aggregate. So it ends up looking only at whatever moz_place ends up first in its result set. If that moz_place's foreign_count is zero, then the whole bookmarkedFragment is zero, even if there are moz_places in the result set that are bookmarked. Change bookmarkedFragment to use TOTAL.

Comment on attachment 9011995 [details] Bug 1489060 - Urlbar autofill is broken when autofilling history is disabled and autofilling bookmarks is enabled Marco Bonardo [::mak] has approved the revision.

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/35b71d287e96 Urlbar autofill is broken when autofilling history is disabled and autofilling bookmarks is enabled r=mak

Please see comment 0 for STR

Marco, do you want to nominate it for Beta backport?

Yes

Approval Request Comment [Feature/Bug causing the regression]: New autofill algorithm, bug 1239708, which landed in 62 [User impact if declined]: Users will see broken behavior (this bug) in 62 and 63 instead of only 62 [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Not yet [Needs manual test from QE? If yes, steps to reproduce]: STR are in comment 0 [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Small two-line change to a SQL query that's used in the specific case described in comment 0 (searching history is disabled, searching bookmarks is enabled) [String changes made/needed]: None

Comment on attachment 9013675 [details] [diff] [review] Beta/63 patch Review of attachment 9013675 [details] [diff] [review]: ----------------------------------------------------------------- Fix for a P1 regression, uplift approved for 63 beta 12, thanks.

I'm going to call this wontfix for 62 as 63 is less than 3 weeks away.

I was able to reproduce this bug on Fx62 (Build ID: 20180830143136) with some URLs (e.g. amazon.com) only. The code fix seems to work fine on the Latest nightly on Windows 10. On mac OS and Ubuntu 16.04 I still see the issue. Please see on Win 10 (Build ID 20181003100127) where code fix is working fine. https://svtest2222-gmail.tinytake.com/sf/Mjk2ODYzM184OTA2NDYy I tried testing CI/treeherder beta build (https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=3947b51293f6f3ddd8173b263cee69f7e4850a45&selectedJob=203066624) and still see the issue https://svtest2222-gmail.tinytake.com/sf/Mjk2ODY1MF84OTA2NDc5 (I see same issue on the Mac OS 10.12, Ubuntu 16.04 with the latest Nightly ((Build ID 20181003100127)) Please let me know if I am missing anything here. Thanks!

You're probably hitting the autofill frecency threshold, which still comes into play for bookmarks and this bug. I tried following the steps shown in your videos (thanks for posting them) using Nightly on macOS, and I too saw that the problem still happens for amazon.com but not for reddit.com. However, if you then visit amazon.com a couple of more times, it starts autofilling. To eliminate the unrelated issue of frecency so that you can verify this bug, you can Clear Recent History (using the default options), and then both amazon.com and reddit.com should autofill. (They do for me.) We have another bug about always autofilling bookmarks regardless of frecency and the threshold, bug 1493636.

(In reply to Drew Willcoxon :adw from comment #24) > You're probably hitting the autofill frecency threshold, which still comes > into play for bookmarks and this bug. I tried following the steps shown in > your videos (thanks for posting them) using Nightly on macOS, and I too saw > that the problem still happens for amazon.com but not for reddit.com. > However, if you then visit amazon.com a couple of more times, it starts > autofilling. > > To eliminate the unrelated issue of frecency so that you can verify this > bug, you can Clear Recent History (using the default options), and then both > amazon.com and reddit.com should autofill. (They do for me.) > > We have another bug about always autofilling bookmarks regardless of > frecency and the threshold, bug 1493636. Thanks for this info! Things are better after clearing history but sometimes after clearing history I have to visit website a couple of more times to get it autofilling. https://svtest2222-gmail.tinytake.com/sf/Mjk3Mjg3OF84OTE3MjQ0 https://svtest2222-gmail.tinytake.com/sf/Mjk3Mjg5NF84OTE3MjYx Apart from the known issue, I verified this bug fix bug for Nightly ( Build ID 20181004100222) and CI beta build (Version 63.0b12, Build ID 20181003092230)on Windows 10, Mac OS 10.12 and Ubuntu 16.04

Thanks for the videos again, they're very helpful. I don't see anything unexpected in them. As you visit amazon and reddit, their frecencies change, and so does the autofill frecency threshold. For example when you visit amazon first at the beginning of the videos, it increases the threshold so that reddit doesn't meet it and it therefore isn't autofilled. Once you start visiting reddit, its frecenc goes up and the threshold changes again. When we're testing with new profiles (or almost new profiles), we're dealing with a very small number of pages in history and a small number of visits, so any visit will have a large effect on the threshold and possibly (or probably) exclude some other page from being autofilled. As for the "never remember history" option, choosing it doesn't clear your current history, so whatever threshold and frecencies that were present when you restart will be present after that, and they'll be preserved until you clear your history.

Not sure if it managed to get in beta 12, but I just updated to beta 12 (received the update a few minutes ago) and I still don't have autofill working.

You probably want bug 1493636, not this one.

Pardon me as I can't understand all the technical comments among the developers here, but as far as what I have described in the very beginning (step to reproduce / actual results / expected results), it hasn't changed one bit even in Firefox 64 - the problem still isn't fixed and is still exactly like how I described in the beginning. May I get a layman update as to what's going on?