417810 - Frequently Visited Pages Disappear From Location Bar Autocomplete

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Aaron Nowack]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3

Intermittently, very frequently visited pages have disappeared from my location bar autocomplete list.  For instance, "http://slashdot.org/" will not appear in the list, even though it is usually the first result as soon as an "s" is typed.  As more of the URL is typed, it still does not appear, although various subpages start to show up.  Manually typing in the full URL and re-visiting the page seems to restore it to its proper place in the auto-complete results, at least until this bug reoccurs.

I have not been able to determine what triggers this; I have only seen it on very frequently visited sites that should be the first autocomplete results, although its possible that I simply haven't noticed it occurring to less visited sites.  I have seen this happen to a group of frequently visited sites all around the same time, but I have also seen it happen to just one, leaving the others unaffected.

Reproducible: Sometimes

Steps to Reproduce:
1. Normal usage of the browser.
2. Start typing a URL into the location bar.
Actual Results:  
The most commonly visited site that is usually the first result for the typed  characters is not present, and does not appear no matter what is typed into the location bar.

Expected Results:  
The usual first result is present.

Comment 1

[Elmar Ludwig]

I think I've seen this as well: I have some URLs that I visit almost daily and during the last two or three weeks it happened several times that some of these seemed to vanish from the automcomplete suggestions. After some trying I found that they suddenly appeared last in the list of autocomplete results (after all the subpages of the site), so they did not completely disappear from the list. Visiting the URL once puts it in front of the results again.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008021304 Minefield/3.0b4pre ID:2008021304

Comment 2

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021603 Minefield/3.0b4pre

WFM so far.

Comment 3

[Robert [minotaur 11]]

I've seen the problem too and wrote about it in the mozillazine forum:
http://forums.mozillazine.org/viewtopic.php?t=629282

> 3.Autocomplete results change their order for a short time

> I have some bookmarks which i visit multiple times daily, so they should
> always appear as first in the autocomplete results, but every few days
> they appear as last for a short time. If this happens they have a frecency
> of -1, which then gets recalculated. I suppose the reason is the expiring 
> of history visits, since my history is more than 90 days
> (browser.history_expire_days_min) old and the number of entries is circa
> 40000 (browser.history_expire_sites).
> It seems that every time a history visit of a specific places entry expires
> the frecency is set to -1. This happens with frequently visited sites quite
> often and increases the probability to hit the short time before
> recalculation.

I'm not shure if my assumption is right, but with the steps below i can reproduce a similar behavior:
1.Set browser.history_expire_days_min to 0 and browser.history_expire_sites to 10
2.Clear the browsing history
3.Visit one bookmark
4.Visit 8 different bookmarks
5.Visit the bookmark from step 3 two times
6.Search for the bookmark from step 3, it should appear as first (frecency 420)
7.Restart Firefox (the visit from step 3 expires)
8.Search for the bookmark from step 3, it appears as last (frecency -1)
9.After the frecency recalculation the bookmark appears again as first (frecency 280)

Comment 4

[Ed Lee :Mardak]

I've noticed this as well, and I looked at it briefly.. Some reason the top ~10? frecency pages get their frecency reset to -1... on migration or some time at browser start? What triggers migration? Would it be b3pre -> b4pre? I couldn't get it to reproduce consistently, and I've took a quick look at the code and didn't find anything.

Sometime about recalculating frecencies?

Comment 5

[Ed Lee :Mardak]

blocking-ff3? This can be somewhat annoying depending on how it actually gets triggered. (Is this something only on trunk because we keep updating every day?)

Comment 6

[Aaron Nowack]

(In reply to comment #5)
> (Is this something only on trunk because we keep updating every
> day?)

I have seen this in Firefox Beta 3, to the best of my recollection once not long after updating from Beta 2 and once later, shortly before filing this bug.

Comment 7

[Ria Klaassen (not reading all bugmail)]

Related to bug 417937 in a way?

Comment 8

[Tony Mechelynck [:tonymec]]

(In reply to comment #7)
> Related to bug 417937 in a way?
> 

Either it isn't, or comment #3 is off the mark, since bug 417937 is about resetting the frecency to zero (don't show) and comment #3 is about resetting it to -1 (must recalc).

Comment 9

[Aaron Nowack]

Just as a data point for the frequency of triggering this bug, I just noticed that this had occurred to at least two sites, roughly three days after the last time.  I have not changed my Firefox install in the interim and am still running Firefox Beta 3, so I don't believe this is related to updating Firefox.

Closing Firefox and restarting had no effect on the problem.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

QA: can you get in touch with Mardak (he's on IRC a lot) and see what sort of testing could help out in terms of getting STR, here? We definitely need to figure this out before we ship.

Comment 11

[Ed Lee :Mardak]

Attachment: v1

I'm still not entirely sure what's causing the problem, but potentially it's something to do with recalculating a frecency of -1. So 4 fixes:

1) Calculate more non -1 frecencies by looking at the original visit date and type for a redirected visit
2) Give non-zero visit bonuses to all visit types
3) Don't replace a valid frecency with -1
4) Pick out places to update frecency at random to avoid getting stuck picking the same places over and over and over again

Comment 12

[Ed Lee :Mardak]

Attachment: v1.1

Fixed up the bonuses now that the query will base the redirect's visit type on the original visit. This will help fix frecencies of -1 when you click a link on a google search result. It happens because you cause a transition_link for the google search result link which then redirects to the actual page.

I also figured out why I was only running into this bug sometimes. Whenever I switch to trunk and not use my adaptive learning builds, I get the frecency of -1 issue. That's because the adaptive learning knows to always show the frequently selected sites.

I might end up breaking this up into smaller patches for separate bugs.

Comment 13

[Ed Lee :Mardak]

Attachment: v1.2

Odd.. I updated the comment but the code wasn't there. Readded the oldFrecency check. Each of these changes should help prevent frecency from ending up at -1.

Comment 14

[Ed Lee :Mardak]

dietrich: Should we try landing these changes to see if it helps avoid the problems for people as we're still not sure what exactly causes the issue?

I suppose people could try these builds and see if frequently visited pages disappear. But then again, we're not sure if it's something made worse by nightly updates.. ?

https://build.mozilla.org/tryserver-builds/2008-02-20_12:[e-mail]-1/

Comment 15

[Dietrich Ayala (:dietrich)]

(In reply to comment #14)
> dietrich: Should we try landing these changes to see if it helps avoid the
> problems for people as we're still not sure what exactly causes the issue?

at this stage, i feel the opposite is true: we shouldn't be making changes without knowing fully why they're being made. without real STR here, it's not an easy sell to accept the patch. have you confirmed that builds w/o adaptive learning but with this patch no longer exhibit the problem?

Comment 16

[Dietrich Ayala (:dietrich)]

Comment on attachment 304343 [details] [diff] [review]
v1.2

>Bug 417810 - Frequently Visited Pages Disappear From Location Bar Autocomplete
>
>diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js
>--- a/browser/app/profile/firefox.js
>+++ b/browser/app/profile/firefox.js
>@@ -647,15 +647,15 @@ pref("places.frecency.thirdBucketWeight"
> pref("places.frecency.thirdBucketWeight", 50);
> pref("places.frecency.fourthBucketWeight", 30);
> pref("places.frecency.defaultBucketWeight", 10);
> 
> // bonus (in percent) for visit transition types for frecency calculations
> pref("places.frecency.embedVisitBonus", 0);
>-pref("places.frecency.linkVisitBonus", 120);
>-pref("places.frecency.typedVisitBonus", 200);
>-pref("places.frecency.bookmarkVisitBonus", 140);
>+pref("places.frecency.linkVisitBonus", 100);
>+pref("places.frecency.typedVisitBonus", 500);
>+pref("places.frecency.bookmarkVisitBonus", 150);

can you explain the rationale and result of these particular gradations in bonus value?

>diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit/components/places/src/nsNavHistory.cpp
>--- a/toolkit/components/places/src/nsNavHistory.cpp
>+++ b/toolkit/components/places/src/nsNavHistory.cpp
>@@ -1010,18 +1010,22 @@ nsNavHistory::InitStatements()
> 
>   // mDBVisitsForFrecency
>   // NOTE: we are not limiting to visits with "visit_type NOT IN (0,4)"
>   // because if we do that, mDBVisitsForFrecency would return no visits
>   // for places with only embed (or undefined) visits.  That would
>   // cause use to estimate a frecency based on what information we do have,
>-  // see CalculateFrecencyInternal().  that would result in a 
>-  // non-zero frecency for a place with only
>-  // embedded visits, instead of a frecency of 0.
>-  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
>-    "SELECT visit_date, visit_type FROM moz_historyvisits " 
>-    "WHERE place_id = ?1 ORDER BY visit_date DESC LIMIT ") +
>+  // see CalculateFrecencyInternal(). That would result in a non-zero frecency
>+  // for a place with only embedded visits, instead of a frecency of 0. If we
>+  // have a temporary or permanent redirect, calculate the frecency as if it
>+  // was the original page visited.
>+  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
>+    "SELECT IFNULL(r.visit_date, v.visit_date) date, IFNULL(r.visit_type, v.visit_type) "
>+    "FROM moz_historyvisits v "
>+    "LEFT OUTER JOIN moz_historyvisits r "
>+      "ON r.id = v.from_visit AND v.visit_type IN (5,6) "

should use the constants from the idl.

r=me on everything except the bonus changes for now.

Comment 17

[Ed Lee :Mardak]

(In reply to comment #16)
> can you explain the rationale and result of these particular gradations in
> bonus value?
Ah that's not as relevant to this bug anymore because before I was giving non-0 bonuses for all visit types. But I implemented the change to use the referrer's visit type and you explained the differences, so I changed them back to 0. Now what remains is just normalizing link visits to 100 and giving more weight to typed results which would better match the expectations of users.. at least some that are pointing out problems of the autocomplete not showing desired results.

> >diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit> >+      "ON r.id = v.from_visit AND v.visit_type IN (5,6) "
> should use the constants from the idl.
Was just following the existing "IN (0,4)" style, but I'll change it to
... + nsPrintfCString("%d,%d", nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT, nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY) + ...

Comment 18

[Ed Lee :Mardak]

I haven't seen the -1 frecencies for frequently visited pages on my debug build. As pointed out in bug 418738 and bug 418739, this patch helps fix the issue by making sure invalid frecencies don't get stuck and keep being recalculated where possible as well as not putting -1 frecencies into the database unnecessarily.

Comment 19

[Dietrich Ayala (:dietrich)]

ok, makes sense. r=me on the bonus changes.

Comment 20

[Ed Lee :Mardak]

Attachment: v1.3

As checked in for this bug. v1.2 has the other changes checked in for bug 418738 and bug 418739.

Comment 21

[Ed Lee :Mardak]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.279; previous revision: 1.278
done
Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.254; previous revision: 1.253
done
Checking in toolkit/components/places/tests/unit/test_000_frecency.js;
/cvsroot/mozilla/toolkit/components/places/tests/unit/test_000_frecency.js,v  <--  test_000_frecency.js
new revision: 1.4; previous revision: 1.3
done

Comment 22

[Robert [minotaur 11]]

I reduced the number of browser.history_expire_sites to 39000 and restarted firefox some times, afterwards many of my frequently visited pages had a frecency of -1. So i still think one reason for the bug is the expiring of history visits (happens regularly for frequently visited sites if the history is at a size limit). Maybe it would be better to directly recalculate the frecency during the expiring of history visits.

Comment 23

[Ed Lee :Mardak]

Ah that makes sense. Expire has..

  // XXX todo
  // forcibly call the "on idle" timer here to do a little work
  // but the rest will happen on idle.

Perhaps we should call UpdateFrecency(placeId) for each?

Comment 24

[Ed Lee :Mardak]

Attachment: v2

Addresses comment #22. Instead of resetting to -1 unconditionally, only reset to -1 if we have no bookmarks and no visits for the url.

Comment 25

[Dietrich Ayala (:dietrich)]

(In reply to comment #22)
> I reduced the number of browser.history_expire_sites to 39000 and restarted
> firefox some times, afterwards many of my frequently visited pages had a
> frecency of -1. So i still think one reason for the bug is the expiring of
> history visits (happens regularly for frequently visited sites if the history
> is at a size limit). Maybe it would be better to directly recalculate the
> frecency during the expiring of history visits.
> 

expiration will remove oldest visits first. unless that's not working correctly, i don't see how your frequently visited sites could be removed.

(In reply to comment #24)
> Created an attachment (id=304733) [details]
> v2
> 
> Addresses comment #22. Instead of resetting to -1 unconditionally, only reset
> to -1 if we have no bookmarks and no visits for the url.
> 

please open a new bug for investigating this expiration+frecency issue.

Comment 26

[Ed Lee :Mardak]

Expiration removes old visits which then does a blanket set frecency to -1 for visits that it removed. So your frequently visited pages will have recent/new visits as well as old/expired visits, and the expiration setting to -1 doesn't care... right now.

Comment 27

[Ed Lee :Mardak]

And that would be another cause for frequently visited pages to disappear from the location bar autocomplete.

Comment 28

[Dietrich Ayala (:dietrich)]

(In reply to comment #26)
> Expiration removes old visits which then does a blanket set frecency to -1 for
> visits that it removed. So your frequently visited pages will have recent/new
> visits as well as old/expired visits, and the expiration setting to -1 doesn't
> care... right now.
> 

(In reply to comment #27)
> And that would be another cause for frequently visited pages to disappear from
> the location bar autocomplete.
> 

ugh, yeah i see now. just re-open this bug then.

Comment 29

[Ed Lee :Mardak]

Comment on attachment 304733 [details] [diff] [review]
v2

r=dietrich from bug 418838

Comment 30

[Ed Lee :Mardak]

Bug 418838 checked in.

Comment 31

[Chris Blore]

VERIFIED Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0