fix for bug #369887: searches in bookmarks sidebar and organize bookmarks dialog don't show all the results: missing 'hidden' bookmarks and failing to match on title.r=dietrich
authorsspitzer@mozilla.org
Thu, 22 Mar 2007 16:05:37 -0700
changeset 5 331cb67f2a3cb141465e0da88f8cd1ef36e85ffc
parent 4 e943454a2e49b2860353c9449359c1822cb14827
child 6 dad02d3ebc7d9e5fdfed17234d31d10e3b1b55ec
push idunknown
push userunknown
push dateunknown
reviewersdietrich
bugs369887
milestone1.9a3pre
fix for bug #369887: searches in bookmarks sidebar and organize bookmarks dialog don't show all the results: missing 'hidden' bookmarks and failing to match on title.r=dietrich
toolkit/components/places/src/nsNavBookmarks.cpp
toolkit/components/places/src/nsNavHistory.cpp
toolkit/components/places/src/nsNavHistoryQuery.h
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@@ -139,17 +139,19 @@ nsNavBookmarks::Init()
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Construct a result where the first columns exactly match those returned by
   // mDBGetURLPageInfo, and additionally contains columns for position,
   // item_child, and folder_child from moz_bookmarks.  This selects only
   // _item_ children which are in moz_places.
   // Results are kGetInfoIndex_*
   NS_NAMED_LITERAL_CSTRING(selectItemChildren,
-    "SELECT h.id, h.url, a.title, h.user_title, h.rev_host, h.visit_count, "
+    "SELECT h.id, h.url, a.title, "
+      "(SELECT title FROM moz_bookmarks WHERE item_child = h.id), "
+      "h.rev_host, h.visit_count, "
       "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
       "f.url, null, a.position, a.item_child, a.folder_child, null, a.id "
     "FROM moz_bookmarks a "
     "JOIN moz_places h ON a.item_child = h.id "
     "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
     "WHERE a.parent = ?1 AND a.position >= ?2 AND a.position <= ?3 ");
 
   // Construct a result where the first columns are padded out to the width
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -576,43 +576,47 @@ nsNavHistory::InitDB(PRBool *aDoImport)
 
 nsresult
 nsNavHistory::InitStatements()
 {
   nsresult rv;
 
   // mDBGetURLPageInfo
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+      "item_child = h.id), h.rev_host, h.visit_count "
       "FROM moz_places h "
       "WHERE h.url = ?1"),
     getter_AddRefs(mDBGetURLPageInfo));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBGetURLPageInfoFull
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+      "item_child = h.id), h.rev_host, h.visit_count, "
         "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
         "f.url "
       "FROM moz_places h "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE h.url = ?1 "),
     getter_AddRefs(mDBGetURLPageInfoFull));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBGetIdPageInfo
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+      "item_child = h.id), h.rev_host, h.visit_count "
       "FROM moz_places h WHERE h.id = ?1"),
                                 getter_AddRefs(mDBGetIdPageInfo));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBGetIdPageInfoFull
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+        "item_child = h.id), h.rev_host, h.visit_count, "
         "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
         "f.url "
       "FROM moz_places h "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE h.id = ?1"),
     getter_AddRefs(mDBGetIdPageInfoFull));
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -667,40 +671,43 @@ nsNavHistory::InitStatements()
       "INSERT INTO moz_places "
       "(url, title, rev_host, hidden, typed, visit_count) "
       "VALUES (?1, ?2, ?3, ?4, ?5, ?6)"),
     getter_AddRefs(mDBAddNewPage));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBVisitToURLResult, should match kGetInfoIndex_* (see GetQueryResults)
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+        "item_child = h.id), h.rev_host, h.visit_count, "
         "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
         "f.url, null "
       "FROM moz_places h "
       "JOIN moz_historyvisits v ON h.id = v.place_id "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE v.id = ?1"),
     getter_AddRefs(mDBVisitToURLResult));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBVisitToVisitResult, should match kGetInfoIndex_* (see GetQueryResults)
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+             "item_child = h.id), h.rev_host, h.visit_count, "
              "v.visit_date, f.url, v.session "
       "FROM moz_places h "
       "JOIN moz_historyvisits v ON h.id = v.place_id "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE v.id = ?1"),
     getter_AddRefs(mDBVisitToVisitResult));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // mDBUrlToURLResult, should match kGetInfoIndex_*
   rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+        "item_child = h.id), h.rev_host, h.visit_count, "
         "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
         "f.url, null "
       "FROM moz_places h "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE h.url = ?1"),
     getter_AddRefs(mDBUrlToUrlResult));
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1866,57 +1873,59 @@ nsNavHistory::GetQueryResults(const nsCO
       sortingMode > nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING) {
     return NS_ERROR_INVALID_ARG;
   }
 
   PRBool asVisits =
     (aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_VISIT ||
      aOptions->ResultType() == nsINavHistoryQueryOptions::RESULTS_AS_FULL_VISIT);
 
-  nsCAutoString commonConditions;
+  nsCAutoString commonConditionsForHistory;
 
   if (! aOptions->IncludeHidden()) {
     // The hiding code here must match the notification behavior in AddVisit
-    commonConditions.AssignLiteral("hidden <> 1 ");
+    commonConditionsForHistory.AssignLiteral("hidden <> 1 ");
 
     // Some items are unhidden but are subframe navigations that we shouldn't
     // show. This happens especially on imported profiles because the previous
     // history system didn't hide as many things as we do now. Some sites,
     // especially Javascript-heavy ones, load things in frames to display them,
     // resulting in a lot of these entries. This filters those visits out.
     if (asVisits)
-      commonConditions.AppendLiteral("AND v.visit_type <> 4 "); // not TRANSITION_EMBED
+      commonConditionsForHistory.AppendLiteral("AND v.visit_type <> 4 "); // not TRANSITION_EMBED
   }
 
   // Query string: Output parameters should be in order of kGetInfoIndex_*
   // WATCH OUT: nsNavBookmarks::Init also creates some statements that share
   // these same indices for passing to RowToResult. If you add something to
   // this, you also need to update the bookmark statements to keep them in
   // sync!
   nsCAutoString queryString;
   nsCAutoString groupBy;
   if (asVisits) {
     // if we want visits, this is easy, just combine all possible matches
     // between the history and visits table and do our query.
     // FIXME(brettw) Add full visit info
     queryString = NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title from moz_bookmarks WHERE "
+             "item_child = h.id), h.rev_host, h.visit_count, "
              "v.visit_date, f.url, v.session "
       "FROM moz_places h "
       "JOIN moz_historyvisits v ON h.id = v.place_id "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE ");
   } else {
     // For URLs, it is more complicated, because we want each URL once. The
     // GROUP BY clause gives us this. To get the max visit time, we populate
     // one column by using a nested SELECT on the visit table. Also, ignore
     // session information.
     // FIXME(brettw) add nulls for full visit info
     queryString = NS_LITERAL_CSTRING(
-      "SELECT h.id, h.url, h.title, h.user_title, h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, h.title, (SELECT title FROM moz_bookmarks WHERE "
+        "item_child = h.id), h.rev_host, h.visit_count, "
         "(SELECT MAX(visit_date) FROM moz_historyvisits WHERE place_id = h.id), "
         "f.url, null "
       "FROM moz_places h "
       "LEFT OUTER JOIN moz_historyvisits v ON h.id = v.place_id "
       "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
       "WHERE ");
     groupBy = NS_LITERAL_CSTRING(" GROUP BY h.id");
   }
@@ -1924,33 +1933,34 @@ nsNavHistory::GetQueryResults(const nsCO
   PRInt32 numParameters = 0;
   nsCAutoString conditions;
   PRInt32 i;
   for (i = 0; i < aQueries.Count(); i ++) {
     nsCString queryClause;
     PRInt32 clauseParameters = 0;
     rv = QueryToSelectClause(aQueries[i], numParameters,
                              &queryClause, &clauseParameters,
-                             commonConditions);
+                             aQueries[i]->OnlyBookmarked() ? 
+                               EmptyCString() : commonConditionsForHistory);
     NS_ENSURE_SUCCESS(rv, rv);
     if (! queryClause.IsEmpty()) {
       if (! conditions.IsEmpty()) // exists previous clause: multiple ones are ORed
         conditions += NS_LITERAL_CSTRING(" OR ");
       conditions += NS_LITERAL_CSTRING("(") + queryClause +
         NS_LITERAL_CSTRING(")");
       numParameters += clauseParameters;
     }
   }
 
   // in cases where there were no queries, we need to use the common conditions
   // (normally these are appended to each clause that are not annotation-based)
   if (! conditions.IsEmpty()) {
     queryString += conditions;
   } else {
-    queryString += commonConditions;
+    queryString += commonConditionsForHistory;
   }
   queryString += groupBy;
 
   // Sort clause: we will sort later, but if it comes out of the DB sorted,
   // our later sort will be basically free. The DB can sort these for free
   // most of the time anyway, because it has indices over these items.
   //
   // FIXME: do some performance tests, I'm not sure that the indices are getting
--- a/toolkit/components/places/src/nsNavHistoryQuery.h
+++ b/toolkit/components/places/src/nsNavHistoryQuery.h
@@ -159,17 +159,17 @@ private:
 
   ~nsNavHistoryQueryOptions() { delete[] mGroupings; }
 
   // IF YOU ADD MORE ITEMS:
   //  * Add a new getter for C++ above if it makes sense
   //  * Add to the serialization code (see nsNavHistory::QueriesToQueryString())
   //  * Add to the deserialization code (see nsNavHistory::QueryStringToQueries)
   //  * Add to the nsNavHistoryQueryOptions::Clone() function
-  //  * Add to the nsNavHistory.cpp::GetSimpleBookmarksQuery function if applicable
+  //  * Add to the nsNavHistory.cpp::GetSimpleBookmarksQueryFolder function if applicable
   PRUint32 mSort;
   PRUint32 mResultType;
   PRUint32 mGroupCount;
   PRUint32 *mGroupings;
   PRBool mExcludeItems;
   PRBool mExcludeQueries;
   PRBool mExcludeReadOnlyFolders;
   PRBool mExpandQueries;