Bug 449406 - bookmarks of TRANSITION_DOWNLOAD and TRANSITION_EMBED don't show up in the location bar if they've been visited

This adds a new function to nsNavBookmarks that determines if a place id is a
"real" bookmark (not a livemark child).  It also makes a number of places use
this new function to unify and simplify a number of code paths.

Additionally, it changes how frecency is calculated for bookmarked items.  If a
visit is bookmarked (aIsBookmarked is true in CalculateFrecencyInternal), the
visited bookmark bonus is now added for all visits.  The visit bonus was also
cut in half for bookmarks so TRANSITION_BOOKMARK will still end up with the same
value (150).

r=dietrich
r=MaK77
This commit is contained in:
Shawn Wilsher 2009-01-27 21:01:33 -05:00
Родитель d440bbfe03
Коммит 30a3eb4e26
9 изменённых файлов: 273 добавлений и 84 удалений

Просмотреть файл

@ -754,7 +754,7 @@ pref("places.frecency.defaultBucketWeight", 10);
pref("places.frecency.embedVisitBonus", 0);
pref("places.frecency.linkVisitBonus", 100);
pref("places.frecency.typedVisitBonus", 2000);
pref("places.frecency.bookmarkVisitBonus", 150);
pref("places.frecency.bookmarkVisitBonus", 75);
pref("places.frecency.downloadVisitBonus", 0);
pref("places.frecency.permRedirectVisitBonus", 0);
pref("places.frecency.tempRedirectVisitBonus", 0);

Просмотреть файл

@ -365,6 +365,23 @@ nsNavBookmarks::InitStatements()
getter_AddRefs(mDBIsBookmarkedInDatabase));
NS_ENSURE_SUCCESS(rv, rv);
// mDBIsRealBookmark
// Checks to make sure a place_id is a bookmark, and isn't a livemark.
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT id "
"FROM moz_bookmarks "
"WHERE fk = ?1 "
"AND type = ?2 "
"AND parent NOT IN ("
"SELECT a.item_id "
"FROM moz_items_annos a "
"JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id "
"WHERE n.name = ?3"
") "
"LIMIT 1"),
getter_AddRefs(mDBIsRealBookmark));
NS_ENSURE_SUCCESS(rv, rv);
// mDBGetLastBookmarkID
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT id "
@ -446,6 +463,7 @@ nsNavBookmarks::FinalizeStatements() {
mDBGetRedirectDestinations,
mDBInsertBookmark,
mDBIsBookmarkedInDatabase,
mDBIsRealBookmark,
mDBGetLastBookmarkID,
mDBSetItemDateAdded,
mDBSetItemLastModified,
@ -890,10 +908,38 @@ nsNavBookmarks::UpdateBookmarkHashOnRemove(PRInt64 aPlaceId)
}
PRBool
nsNavBookmarks::IsRealBookmark(PRInt64 aPlaceId)
{
// Fast path is to check the hash table first. If it is in the hash table,
// then verify that it is a real bookmark.
PRInt64 bookmarkId;
PRBool isBookmark = mBookmarksHash.Get(aPlaceId, &bookmarkId);
if (!isBookmark)
return PR_FALSE;
{
mozStorageStatementScoper scope(mDBIsRealBookmark);
(void)mDBIsRealBookmark->BindInt64Parameter(0, aPlaceId);
(void)mDBIsRealBookmark->BindInt32Parameter(1, TYPE_BOOKMARK);
(void)mDBIsRealBookmark->BindUTF8StringParameter(
2, NS_LITERAL_CSTRING(LMANNO_FEEDURI)
);
// If we get any rows, then there exists at least one bookmark corresponding
// to aPlaceId that is not a livemark item.
if (NS_SUCCEEDED(mDBIsRealBookmark->ExecuteStep(&isBookmark)))
return isBookmark;
}
return PR_FALSE;
}
// nsNavBookmarks::IsBookmarkedInDatabase
//
// This checks to see if the specified URI is actually bookmarked, bypassing
// our hashtable. Normal IsBookmarked checks just use the hashtable.
// This checks to see if the specified place_id is actually bookmarked.
// This does not check for redirects in the hashtable.
nsresult
nsNavBookmarks::IsBookmarkedInDatabase(PRInt64 aPlaceId,
@ -2577,42 +2623,8 @@ nsNavBookmarks::ChangeBookmarkURI(PRInt64 aBookmarkId, nsIURI *aNewURI)
// is: find all bookmarks corresponding to oldPlaceId that are not livemark
// items, i.e., whose parents are not livemarks. If any such bookmarks exist,
// oldPlaceId is still bookmarked.
PRBool isBookmarked = PR_FALSE;
nsCOMPtr<mozIStorageStatement> isBookmarkedStmt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT id "
"FROM moz_bookmarks "
"WHERE fk = ?1 AND "
"type = ?2 AND "
"parent NOT IN ("
"SELECT a.item_id "
"FROM moz_items_annos a "
"JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id "
"WHERE n.name = ?3"
")"),
getter_AddRefs(isBookmarkedStmt));
NS_ENSURE_SUCCESS(rv, rv);
{
mozStorageStatementScoper scope(isBookmarkedStmt);
rv = isBookmarkedStmt->BindInt64Parameter(0, oldPlaceId);
NS_ENSURE_SUCCESS(rv, rv);
rv = isBookmarkedStmt->BindInt32Parameter(1, TYPE_BOOKMARK);
NS_ENSURE_SUCCESS(rv, rv);
rv = isBookmarkedStmt->BindUTF8StringParameter(
2, NS_LITERAL_CSTRING(LMANNO_FEEDURI));
NS_ENSURE_SUCCESS(rv, rv);
// If executing isBookmarkedStmt returns any rows, then there exists at least
// one bookmark corresponding to oldPlaceId that is not a livemark item.
// isBookmarked will be set to true in that case and false otherwise.
rv = isBookmarkedStmt->ExecuteStep(&isBookmarked);
NS_ENSURE_SUCCESS(rv, rv);
}
rv = History()->UpdateFrecency(oldPlaceId, isBookmarked);
rv = History()->UpdateFrecency(oldPlaceId, IsRealBookmark(oldPlaceId));
NS_ENSURE_SUCCESS(rv, rv);
nsCAutoString spec;

Просмотреть файл

@ -96,6 +96,15 @@ public:
PRInt32* aIndex,
PRInt64* aNewFolder);
/**
* Determines if we have a real bookmark or not (not a livemark).
*
* @param aPlaceId
* The place_id of the location to check against.
* @returns true if it's a real bookmark, false otherwise.
*/
PRBool IsRealBookmark(PRInt64 aPlaceId);
// Called by History service when quitting.
nsresult OnQuit();
@ -225,6 +234,7 @@ private:
nsCOMPtr<mozIStorageStatement> mDBGetRedirectDestinations;
nsCOMPtr<mozIStorageStatement> mDBInsertBookmark;
nsCOMPtr<mozIStorageStatement> mDBIsBookmarkedInDatabase;
nsCOMPtr<mozIStorageStatement> mDBIsRealBookmark;
nsCOMPtr<mozIStorageStatement> mDBGetLastBookmarkID;
nsCOMPtr<mozIStorageStatement> mDBSetItemDateAdded;
nsCOMPtr<mozIStorageStatement> mDBSetItemLastModified;

Просмотреть файл

@ -1311,12 +1311,6 @@ nsNavHistory::InitStatements()
getter_AddRefs(mDBGetPlaceVisitStats));
NS_ENSURE_SUCCESS(rv, rv);
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT b.parent FROM moz_bookmarks b "
"WHERE b.type = 1 AND b.fk = ?1"),
getter_AddRefs(mDBGetBookmarkParentsForPlace));
NS_ENSURE_SUCCESS(rv, rv);
// when calculating frecency, we want the visit count to be
// all the visits.
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
@ -2760,7 +2754,8 @@ nsNavHistory::AddVisit(nsIURI* aURI, PRTime aTime, nsIURI* aReferringURI,
// Update frecency (*after* the visit info is in the db)
// Swallow errors here, since if we've gotten this far, it's more
// important to notify the observers below.
(void)UpdateFrecency(pageID, PR_FALSE);
nsNavBookmarks *bs = nsNavBookmarks::GetBookmarksService();
(void)UpdateFrecency(pageID, bs->IsRealBookmark(pageID));
// Notify observers: The hidden detection code must match that in
// GetQueryResults to maintain consistency.
@ -7089,6 +7084,14 @@ nsNavHistory::CalculateFrecencyInternal(PRInt64 aPlaceId,
break;
}
// Always add the bookmark visit bonus.
if (aIsBookmarked)
bonus += mBookmarkVisitBonus;
#ifdef DEBUG_FRECENCY
printf("CalculateFrecency() for place %lld has a bonus of %d\n", aPlaceId, bonus);
#endif
// if bonus was zero, we can skip the work to determine the weight
if (bonus) {
PRTime visitDate = mDBVisitsForFrecency->AsInt64(0);
@ -7182,7 +7185,7 @@ nsNavHistory::CalculateFrecencyInternal(PRInt64 aPlaceId,
return NS_OK;
}
nsresult
nsresult
nsNavHistory::CalculateFrecency(PRInt64 aPlaceId,
PRInt32 aTyped,
PRInt32 aVisitCount,
@ -7191,46 +7194,17 @@ nsNavHistory::CalculateFrecency(PRInt64 aPlaceId,
{
*aFrecency = 0;
nsresult rv;
nsCOMPtr<nsILivemarkService> lms =
do_GetService(NS_LIVEMARKSERVICE_CONTRACTID, &rv);
NS_ENSURE_SUCCESS(rv, rv);
PRBool isBookmark = PR_FALSE;
// determine if the place is a (non-livemark item) bookmark and prevent
// place: queries from showing up in the URL bar autocomplete results
if (!IsQueryURI(aURL) && aPlaceId != -1) {
mozStorageStatementScoper scope(mDBGetBookmarkParentsForPlace);
rv = mDBGetBookmarkParentsForPlace->BindInt64Parameter(0, aPlaceId);
NS_ENSURE_SUCCESS(rv, rv);
// this query can return multiple parent folders because
// it is possible for the user to bookmark something that
// is also a livemark item
PRBool hasMore = PR_FALSE;
while (NS_SUCCEEDED(mDBGetBookmarkParentsForPlace->ExecuteStep(&hasMore))
&& hasMore) {
PRInt64 folderId;
rv = mDBGetBookmarkParentsForPlace->GetInt64(0, &folderId);
NS_ENSURE_SUCCESS(rv, rv);
PRBool parentIsLivemark;
rv = lms->IsLivemark(folderId, &parentIsLivemark);
NS_ENSURE_SUCCESS(rv, rv);
// we found one parent that is not a livemark feed, so we can stop
if (!parentIsLivemark) {
isBookmark = PR_TRUE;
break;
}
}
nsNavBookmarks *bs = nsNavBookmarks::GetBookmarksService();
isBookmark = bs->IsRealBookmark(aPlaceId);
}
rv = CalculateFrecencyInternal(aPlaceId, aTyped, aVisitCount,
isBookmark, aFrecency);
nsresult rv = CalculateFrecencyInternal(aPlaceId, aTyped, aVisitCount,
isBookmark, aFrecency);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
@ -7550,7 +7524,6 @@ nsNavHistory::FinalizeStatements() {
mDBVisitsForFrecency,
mDBUpdateFrecencyAndHidden,
mDBGetPlaceVisitStats,
mDBGetBookmarkParentsForPlace,
mDBFullVisitCount,
mDBInvalidFrecencies,
mDBOldFrecencies,

Просмотреть файл

@ -467,7 +467,6 @@ protected:
nsCOMPtr<mozIStorageStatement> mDBVisitsForFrecency;
nsCOMPtr<mozIStorageStatement> mDBUpdateFrecencyAndHidden;
nsCOMPtr<mozIStorageStatement> mDBGetPlaceVisitStats;
nsCOMPtr<mozIStorageStatement> mDBGetBookmarkParentsForPlace;
nsCOMPtr<mozIStorageStatement> mDBFullVisitCount;
mozIStorageStatement *GetDBInvalidFrecencies();
nsCOMPtr<mozIStorageStatement> mDBInvalidFrecencies;

Просмотреть файл

@ -86,3 +86,56 @@ function clearDB() {
} catch(ex) { dump("Exception: " + ex); }
}
clearDB();
/**
* Dumps the rows of a table out to the console.
*
* @param aName
* The name of the table or view to output.
*/
function dump_table(aName)
{
let db = Cc["@mozilla.org/browser/nav-history-service;1"].
getService(Ci.nsPIPlacesDatabase).
DBConnection;
let stmt = db.createStatement("SELECT * FROM " + aName);
dump("\n*** Printing data from " + aName + ":\n");
let count = 0;
while (stmt.executeStep()) {
let columns = stmt.numEntries;
if (count == 0) {
// print the column names
for (let i = 0; i < columns; i++)
dump(stmt.getColumnName(i) + "\t");
dump("\n");
}
// print the row
for (let i = 0; i < columns; i++) {
switch (stmt.getTypeOfIndex(i)) {
case Ci.mozIStorageValueArray.VALUE_TYPE_NULL:
dump("NULL\t");
break;
case Ci.mozIStorageValueArray.VALUE_TYPE_INTEGER:
dump(stmt.getInt64(i) + "\t");
break;
case Ci.mozIStorageValueArray.VALUE_TYPE_FLOAT:
dump(stmt.getDouble(i) + "\t");
break;
case Ci.mozIStorageValueArray.VALUE_TYPE_TEXT:
dump(stmt.getString(i) + "\t");
break;
}
}
dump("\n");
count++;
}
dump("*** There were a total of " + count + " rows of data.\n\n");
stmt.reset();
stmt.finalize();
stmt = null;
}

Просмотреть файл

@ -40,6 +40,15 @@
*/
Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
const TRANSITION_LINK = Ci.nsINavHistoryService.TRANSITION_LINK;
const TRANSITION_TYPED = Ci.nsINavHistoryService.TRANSITION_TYPED;
const TRANSITION_BOOKMARK = Ci.nsINavHistoryService.TRANSITION_BOOKMARK;
const TRANSITION_EMBED = Ci.nsINavHistoryService.TRANSITION_EMBED;
const TRANSITION_REDIRECT_PERMANENT = Ci.nsINavHistoryService.TRANSITION_REDIRECT_PERMANENT;
const TRANSITION_REDIRECT_TEMPORARY = Ci.nsINavHistoryService.TRANSITION_REDIRECT_TEMPORARY;
const TRANSITION_DOWNLOAD = Ci.nsINavHistoryService.TRANSITION_DOWNLOAD;
let current_test = 0;
function AutoCompleteInput(aSearches) {
@ -164,7 +173,56 @@ let gDate = new Date(Date.now() - 1000 * 60 * 60) * 1000;
// Store the page info for each uri
let gPages = [];
function addPageBook(aURI, aTitle, aBook, aTags, aKey)
/**
* Sets the page title synchronously. The page must already be in the database.
*
* @param aURI
* An nsIURI to set the title for.
* @param aTitle
* The title to set the page to.
*/
function setPageTitle(aURI, aTitle)
{
// XXX this function only exists because we have no API to do this. It should
// be added in bug 421897.
let db = histsvc.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
let stmt = db.createStatement(
"UPDATE moz_places_view " +
"SET title = :title " +
"WHERE url = :uri"
);
stmt.params.title = aTitle;
stmt.params.uri = aURI.spec;
stmt.execute();
stmt.finalize();
}
/**
* Adds a page, and creates various properties for it depending on the
* parameters passed in. This function will also add one visit.
*
* @param aURI
* An index into kURIs that holds the string for the URI we are to add a
* page for.
* @param aTitle
* An index into kTitles that holds the string for the title we are to
* associate with the specified URI.
* @param aBook [optional]
* An index into kTitles that holds the string for the title we are to
* associate with the bookmark. If this is undefined, no bookmark is
* created.
* @param aTags [optional]
* An array of indexes into kTitles that hold the strings for the tags we
* are to associate with the URI. If this is undefined (or aBook is), no
* tags are added.
* @param aKey [optional]
* A string to associate as the keyword for this bookmark. aBook must be
* a valid index into kTitles for this to be checked and used.
* @param aTransitionType [optional]
* The transition type to use when adding the visit. The default is
* nsINavHistoryService::TRANSITION_LINK.
*/
function addPageBook(aURI, aTitle, aBook, aTags, aKey, aTransitionType)
{
// Add a page entry for the current uri
gPages[aURI] = [aURI, aBook != undefined ? aBook : aTitle, aTags];
@ -176,8 +234,12 @@ function addPageBook(aURI, aTitle, aBook, aTags, aKey)
out.push("\nuri=" + kURIs[aURI]);
out.push("\ntitle=" + title);
// Add the page and a visit for good measure
bhist.addPageWithDetails(uri, title, gDate);
// Add the page and a visit
let tt = aTransitionType || TRANSITION_LINK;
let isRedirect = tt == TRANSITION_REDIRECT_PERMANENT ||
tt == TRANSITION_REDIRECT_TEMPORARY;
histsvc.addVisit(uri, gDate, null, tt, isRedirect, 0);
setPageTitle(uri, title);
// Add a bookmark if we need to
if (aBook != undefined) {

Просмотреть файл

@ -0,0 +1,76 @@
/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
* vim:set ts=2 sw=2 sts=2 et:
* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
* 1.1 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* http://www.mozilla.org/MPL/
*
* Software distributed under the License is distributed on an "AS IS" basis,
* WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
* for the specific language governing rights and limitations under the
* License.
*
* The Original Code is Places Test Code.
*
* The Initial Developer of the Original Code is
* Mozilla Corporation.
* Portions created by the Initial Developer are Copyright (C) 2009
* the Initial Developer. All Rights Reserved.
*
* Contributor(s):
* Shawn Wilsher <me@shawnwilsher.com> (Original Author)
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
* the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
* in which case the provisions of the GPL or the LGPL are applicable instead
* of those above. If you wish to allow use of your version of this file only
* under the terms of either the GPL or the LGPL, and not to allow others to
* use your version of this file under the terms of the MPL, indicate your
* decision by deleting the provisions above and replace them with the notice
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
/**
* Tests bug 449406 to ensure that TRANSITION_DOWNLOAD and TRANSITION_EMBED
* bookmarked uri's show up in the location bar.
*/
// Define some shared uris and titles (each page needs its own uri)
let kURIs = [
"http://download/bookmarked",
"http://embed/bookmarked",
"http://download",
"http://embed",
];
let kTitles = [
"download-bookmark",
"embed-bookmark",
"download2",
"embed2",
];
// Add download and embed uris
addPageBook(0, 0, 0, undefined, undefined, TRANSITION_DOWNLOAD);
addPageBook(1, 1, 1, undefined, undefined, TRANSITION_EMBED);
addPageBook(2, 2, undefined, undefined, undefined, TRANSITION_DOWNLOAD);
addPageBook(3, 3, undefined, undefined, undefined, TRANSITION_EMBED);
// Provide for each test: description; search terms; array of gPages indices of
// pages that should match; optional function to be run before the test
let gTests = [
["0: Searching for bookmarked download uri matches",
kTitles[0], [0]],
["1: Searching for bookmarked embed uri matches",
kTitles[1], [1]],
["2: Searching for download uri does not match",
kTitles[2], []],
["3: Searching for embed uri does not match",
kTitles[3], []],
];

Просмотреть файл

@ -145,6 +145,10 @@ bucketPrefs.every(function(bucket) {
}
else {
// visited
// visited bookmarks get the visited bookmark bonus twice
if (visitType == Ci.nsINavHistoryService.TRANSITION_BOOKMARK)
bonusValue = bonusValue * 2;
var points = Math.ceil(1 * ((bonusValue / parseFloat(100.000000)).toFixed(6) * weight) / 1);
if (!points) {
if (!visitType ||