Open Bug 1608855 Opened 6 years ago Updated 5 years ago

url bar suggesting an url alphabetically, not sorted by most visited

Categories

(Firefox :: Address Bar, defect, P5)

Product:
Firefox
Component:
Address Bar
Version:
73 Branch
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox73 --- affected

People

(Reporter: e412byoy7, Unassigned)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:73.0) Gecko/20100101 Firefox/73.0

Steps to reproduce:

have youtube.com and y2mate.com as bookmarks. Then open new tab, enter "y

Actual results:

it will ALWAYS suggest y2mate.com instead of youtube.com, even though I super regularly visit youtube, and barely y2mate.com

Expected results:

youtube.com should be on top as top suggestion

status-firefox73: --- → affected
Component: Untriaged → Address Bar

I can't reproduce this. Could you please post a screenshot of what you're seeing?

Flags: needinfo?(danielboontje)
Priority: -- → P5

https://i.postimg.cc/3rqsTsTk/Y-letter-suggestion-bug.jpg

Flags: needinfo?(danielboontje)
Flags: needinfo?(adw)

Thanks. I think I may know what's happening, but I'm not sure yet. Could you please do these steps and paste the output here in the bug?

  1. Open about:config and set the devtools.chrome.enabled preference to true
  2. Open the browser console (Tools > Web Developer > Browser Console)
  3. Paste in the following code and press the enter key, and then please copy and paste the output here in the bug.

Code:

(async function () {
  let db = await PlacesUtils.promiseDBConnection();
  let rows = await db.execute(`
    WITH s(count, sum, squares) AS (
      SELECT
        CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_count") AS REAL),
        CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_sum") AS REAL),
        CAST((SELECT IFNULL(value, 0.0) FROM moz_meta WHERE key = "origin_frecency_sum_of_squares") AS REAL)
    )
    SELECT count, sum, squares, CASE count
    WHEN 0 THEN 0.0
    WHEN 1 THEN sum
    ELSE (sum / count) + (0 * sqrt((squares - ((sum * sum) / count)) / count))
    END
    FROM s
  `);
  for (let i = 0; i < 4; i++) {
    console.log(rows[0].getResultByIndex(i));
  }
  rows = await db.execute(`
    SELECT frecency, prefix, host
    FROM moz_origins
    WHERE like("%youtube.com", host) OR like("%y2mate.com", host)
  `);
  for (let row of rows) {
    console.log(row.getResultByIndex(0),
                row.getResultByIndex(1),
                row.getResultByIndex(2));
  }
})();
Flags: needinfo?(adw)

ReferenceError: PlacesUtils is not defined

Flags: needinfo?(adw)

Did you run the script in the Browser Console (ctrl+shift+j)? The normal devtools console won't work.

Flags: needinfo?(adw) → needinfo?(danielboontje)

oooh that was it. Here we go:

Promise { <state>: "pending" }

11 debugger eval code:18:13
53982 debugger eval code:18:13
725885264 debugger eval code:18:13
4907.454545454545 debugger eval code:18:13
-19625 https:// www.youtube.com debugger eval code:26:13
-20076 http:// www.youtube.com debugger eval code:26:13
-1172 https:// gaming.youtube.com debugger eval code:26:13
-643 https:// www.y2mate.com debugger eval code:26:13

Flags: needinfo?(danielboontje) → needinfo?(adw)

Thanks. The first thing that jumps out is that the frecency values for all those domains are negative, which shouldn't happen... I wonder how they got into that state. I would need to think about it. The y2mate value is greater than the others, and that's why it's autofilled instead of youtube, but if you're visiting youtube more often, it should be greater. I'm guessing the fact that it's not is related to the same problem that caused the frecencies to be negative in the first place.

Flags: needinfo?(adw)

Do you do anything "unusual" with your Firefox history? Like clear your history often, or use permanent private browsing mode, or clear history when Firefox closes? Have you changed any history-related hidden preferences? Do you use Firefox Sync? I'm just trying to think of things that might affect this.

hm, the new Sync bookmarks engine may set frecency to a negative value
https://searchfox.org/mozilla-central/source/toolkit/components/places/bookmark_sync/src/store.rs#875
https://searchfox.org/mozilla-central/source/toolkit/components/places/bookmark_sync/src/store.rs#943
This is something I always wanted to change, indeed one of the first scopes for rewriting frecency is having a valid_frecency bool column.

The fact is then we should recalculate these frecencies in DecayFrecency()
https://searchfox.org/mozilla-central/source/toolkit/components/places/nsNavHistory.cpp#240
But this runs on idle-daily, and we know some of our users fail to hit idle due to bugs like bug 973591 or bug 1603900. This may be one of those cases.

If you go back into about:support and look for the "Important Modified Preferences" section, what is the value of idle.lastDailyNotification in that list?

Flags: needinfo?(danielboontje)

@Drew yes I clear it often, yes I use Sync.

@Marco "idle.lastDailyNotification" value is 1579725490 (for readability, 1.579.725.490, a number that's over one billion)

Flags: needinfo?(danielboontje) → needinfo?(mak)

interesting, this is just yesterday, so it should have fixed those frecency values...

Flags: needinfo?(mak)

How do we proceed now? :/
Does this data help?

places.history.expiration.transient_current_max_pages = 72474

Flags: needinfo?(adw)

Could you please run again the script today? I'm trying to understand if you're stuck with negative frecency, or if Sync is just too aggressive invalidating.

Could you please also run Places Database Integrity check from the about:support page, and report here the output?

Flags: needinfo?(adw) → needinfo?(danielboontje)

"SyntaxError: missing ) after for-loop control"

that comes with the above code.

Integrity check:

Task: checkIntegrity

  • The places.sqlite database is sane
  • The favicons.sqlite database is sane

Task: invalidateCaches

  • The caches have been invalidated

Task: checkCoherence

  • The database is coherent

Task: expire

  • Database cleaned up

Task: originFrecencyStats

  • Recalculated origin frecency stats

Task: vacuum

  • Initial database size is 5120KiB
  • The database has been vacuumed
  • Final database size is 5120KiB

Task: stats

  • Places.sqlite size is 5120KiB
  • Favicons.sqlite size is 1792KiB
  • pragma_user_version is 53
  • pragma_page_size is 32768
  • pragma_cache_size is -2048
  • pragma_journal_mode is wal
  • pragma_synchronous is 1
  • History can store a maximum of 112348 unique pages
  • Table moz_origins has 589 records
  • Table moz_places has 1406 records
  • Table moz_historyvisits has 344 records
  • Table moz_inputhistory has 104 records
  • Table moz_bookmarks has 7060 records
  • Table moz_bookmarks_deleted has 0 records
  • Table moz_keywords has 22 records
  • Table sqlite_sequence has 1 records
  • Table moz_anno_attributes has 2 records
  • Table moz_annos has 76 records
  • Table moz_items_annos has 0 records
  • Table moz_meta has 6 records
  • Table sqlite_stat1 has 19 records
  • Index sqlite_autoindex_moz_origins_1
  • Index sqlite_autoindex_moz_inputhistory_1
  • Index sqlite_autoindex_moz_bookmarks_deleted_1
  • Index sqlite_autoindex_moz_keywords_1
  • Index sqlite_autoindex_moz_anno_attributes_1
  • Index moz_places_url_hashindex
  • Index moz_places_hostindex
  • Index moz_places_visitcount
  • Index moz_places_frecencyindex
  • Index moz_places_lastvisitdateindex
  • Index moz_places_guid_uniqueindex
  • Index moz_places_originidindex
  • Index moz_historyvisits_placedateindex
  • Index moz_historyvisits_fromindex
  • Index moz_historyvisits_dateindex
  • Index moz_bookmarks_itemindex
  • Index moz_bookmarks_parentindex
  • Index moz_bookmarks_itemlastmodifiedindex
  • Index moz_bookmarks_dateaddedindex
  • Index moz_bookmarks_guid_uniqueindex
  • Index moz_keywords_placepostdata_uniqueindex
  • Index moz_annos_placeattributeindex
  • Index moz_items_annos_itemattributeindex

Task: _refreshUI

Flags: needinfo?(danielboontje) → needinfo?(mak)

(In reply to Dan from comment #14)

"SyntaxError: missing ) after for-loop control"

you ran the same code in comment 6, how can it be broken now? Maybe there was a copy paste mistake?

Flags: needinfo?(mak)

Doubt it, ran it twice. In comment 6 I ran it on my Laptop (in Firefox Beta I think), today (my Laptop not with me) I ran it on my desktop PC, Firefox Developer edition.

(In reply to Dan from comment #14)

"SyntaxError: missing ) after for-loop control"

I see this too when copying the code above, but in that case I don't know why it worked for you the first time. O_o

The problem is that the < is replaced with &lt; when you paste. I don't know how to work around that. So could you please search and replace the &lt; with < ?

I ran it in the wrong console the 2nd time I think, but anyways, I removed y2mate, I was really getting annoyed by this bug. Do you still want me to re-add y2mate to bookmarks just to hunt down the cause of this strange bug/is this bugfix very important to you/how important is it to the mozilla community?

Well, if Sync for example is causing this problem for you, and/or frecency recalculation isn't fixing it, it's probably a problem for other people too, so we should fix it. So far in this bug it seems that autofill is working as designed, and there's some external problem that's interfering with it.

I don't think there's any more information we need from you right now, but maybe Marco can think of something. My thought is that we would need to try to reason about how you ended up in this state. It's possible Sync is interfering, or maybe Places isn't keeping autofill frecencies updated correctly in some case.

Maybe we should add a telemetry probe for counting how many autofill frecencies are negative, to see how widespread a problem this is.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.