Closed
Bug 412110
Opened 17 years ago
Closed 10 years ago
don't bother calculating frecencies for place: urls (they should be 0)
Categories
(Toolkit :: Places, defect, P5)
Toolkit
Places
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: sspitzer, Unassigned)
References
Details
don't both calculating frecencies for place: urls
the fix will be to nsNavHistory::InternalAddNewPage()
we currently do:
// frecency
PRInt32 frecency = -1;
if (aCalculateFrecency) {
rv = CalculateFrecency(aTyped, aVisitCount,
-1 /* no page id, since this page doesn't exist */,
url,
&frecency);
NS_ENSURE_SUCCESS(rv, rv);
}
I think we should do something like:
PRInt32 frecency = -1;
if (IsQueryURL(url))
frecency = 0; // place: bookmarks should not show up in autocomplete
else if (aCalculateFrecency) {
rv = CalculateFrecency(aTyped, aVisitCount,
-1 /* no page id, since this page doesn't exist */,
url,
&frecency);
NS_ENSURE_SUCCESS(rv, rv);
}
Note that if aCalculateFrecency is false, we use -1 for frecency to show that it is invalid, and need to be calculated (on idle) but it will still show up in autocomplete.
for place: urls, frececny should always be 0, to prevent them from showing up in autocomplete.
note, even when we fix this, we still need the code in FixInvalidFrecenciesForExcludedPlaces() to fix place: queries, as we might have set a place: query to -1 on clear all private data or from migration from fx 3b2.
another note: it might be possible that something is calling CalculateFrecencyInternal() directly with a place: url.
Here's a stack showing how this can happen:
places.dll!nsNavHistory::CalculateFrecencyInternal(int aTyped=0, int aVisitCount=0, __int64 aPlaceId=-1, int aIsBookmarked=0, int * aFrecency=0x0012c468) Line 6123 C++
places.dll!nsNavHistory::CalculateFrecency(int aTyped=0, int aVisitCount=0, __int64 aPlaceId=-1, nsCAutoString & aURL={...}, int * aFrecency=0x0012c468) Line 6196 + 0x20 bytes C++
> places.dll!nsNavHistory::InternalAddNewPage(nsIURI * aURI=0x00fb5c10, const nsAString_internal & aTitle={...}, int aHidden=1, int aTyped=0, int aVisitCount=0, int aCalculateFrecency=1, __int64 * aPageID=0x0012c740) Line 1481 + 0x25 bytes C++
places.dll!nsNavHistory::GetUrlIdFor(nsIURI * aURI=0x00fb5c10, __int64 * aEntryID=0x0012c740, int aAutoCreate=1) Line 1408 + 0x1c bytes C++
places.dll!nsNavBookmarks::InsertBookmark(__int64 aFolder=8, nsIURI * aItem=0x00fb5c10, int aIndex=-1, const nsACString_internal & aTitle={...}, __int64 * aNewBookmarkId=0x0012c988) Line 874 + 0x19 bytes C++
Reporter | ||
Updated•17 years ago
|
Summary: don't both calculating frecencies for place: urls → doncalculating frecencies for place: urls
Reporter | ||
Updated•17 years ago
|
Summary: doncalculating frecencies for place: urls → don't bother calculating frecencies for place: urls (they should be 0)
Comment 1•17 years ago
|
||
here's an example of where we call CalculateFrecenyInternal() directly:
places.dll!nsNavHistory::CalculateFrecencyInternal(int aTyped=0, int aVisitCount=0, __int64 aPlaceId=51, int aIsBookmarked=0, int * aFrecency=0x0012c55c) Line 6121 C++
places.dll!nsNavHistory::UpdateFrecency(__int64 aPlaceId=51, int aIsBookmarked=0) Line 5999 + 0x23 bytes C++
> places.dll!nsNavBookmarks::InsertBookmark(__int64 aFolder=60, nsIURI * aItem=0x069d2488, int aIndex=-1, const nsACString_internal & aTitle={...}, __int64 * aNewBookmarkId=0x0012c988) Line 947 + 0x1b bytes C++
That happens when creating a bookmark.
Comment 2•17 years ago
|
||
this will also happen when you change a place: bookmark
places.dll!nsNavHistory::CalculateFrecencyInternal(int aTyped=0, int aVisitCount=0, __int64 aPlaceId=53, int aIsBookmarked=1, int * aFrecency=0x00129478) Line 6121 C++
places.dll!nsNavHistory::UpdateFrecency(__int64 aPlaceId=53, int aIsBookmarked=1) Line 5999 + 0x23 bytes C++
> places.dll!nsNavBookmarks::ChangeBookmarkURI(__int64 aBookmarkId=10, nsIURI * aNewURI=0x06c8a1b0) Line 2197 + 0x19 bytes C++
Updated•16 years ago
|
Priority: -- → P5
Product: Firefox → Toolkit
QA Contact: places → places
Comment 3•10 years ago
|
||
the pointed out code is quite old, I think nowadays (I quickly went through the code and looks like I'm recalling correctly) we are doing the right thing (we set frecency to 0 and never update 0 frecencies).
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•