Bug 536081 - Can't delete all history entries returned by a search in Library or Sidebar. r=dietrich

toolkit/components/places/src/History.cpp

@@ -82,7 +82,7 @@ public:
     nsresult rv = aURI->GetSpec(spec);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = stmt->BindUTF8StringParameter(0, spec);
+    rv = BindStatementURLCString(stmt, 0, spec);
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsRefPtr<VisitedQuery> callback = new VisitedQuery(aURI);

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -728,7 +728,7 @@ nsNavBookmarks::IsRealBookmark(PRInt64 aPlaceId)
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
   rv = stmt->BindInt32Parameter(1, TYPE_BOOKMARK);
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
-  rv = stmt->BindUTF8StringParameter(2, NS_LITERAL_CSTRING(LMANNO_FEEDURI));
+  rv = BindStatementURLCString(stmt, 2, NS_LITERAL_CSTRING(LMANNO_FEEDURI));
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
 
   // If we get any rows, then there exists at least one bookmark corresponding

toolkit/components/places/src/nsNavHistory.cpp

@@ -2009,7 +2009,7 @@ PRBool nsNavHistory::IsURIStringVisited(const nsACString& aURIString)
 
   // check the main DB
   mozStorageStatementScoper scoper(mDBIsPageVisited);
-  nsresult rv = mDBIsPageVisited->BindUTF8StringParameter(0, aURIString);
+  nsresult rv = BindStatementURLCString(mDBIsPageVisited, 0, aURIString);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
 
   PRBool hasMore = PR_FALSE;
@@ -2531,7 +2531,7 @@ nsNavHistory::FixInvalidFrecenciesForExcludedPlaces()
     getter_AddRefs(dbUpdateStatement));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = dbUpdateStatement->BindUTF8StringParameter(0, NS_LITERAL_CSTRING(LMANNO_FEEDURI));
+  rv = BindStatementURLCString(dbUpdateStatement, 0, NS_LITERAL_CSTRING(LMANNO_FEEDURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = dbUpdateStatement->Execute();
@@ -6332,8 +6332,7 @@ nsNavHistory::BindQueryClauseParameters(mozIStorageStatement* statement,
       nsCAutoString uriString;
       aQuery->Uri()->GetSpec(uriString);
       uriString.Append(char(0x7F)); // MAX_UTF8
-      rv = statement->BindUTF8StringParameter(index.For("uri_upper"),
-        StringHead(uriString, URI_LENGTH_MAX));
+      rv = BindStatementURLCString(statement, index.For("uri_upper"), uriString);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
@@ -6569,7 +6568,8 @@ nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aQueryNode,
         // Convert title and url for the current node to UTF16 strings.
         NS_ConvertUTF8toUTF16 nodeTitle(aSet[nodeIndex]->mTitle);
         // Unescape the URL for search terms matching.
-        NS_ConvertUTF8toUTF16 nodeURL(NS_UnescapeURL(aSet[nodeIndex]->mURI));
+        nsCAutoString cNodeURL(aSet[nodeIndex]->mURI);
+        NS_ConvertUTF8toUTF16 nodeURL(NS_UnescapeURL(cNodeURL));
 
         // Determine if every search term matches anywhere in the title, url or
         // tag.
@@ -7329,7 +7329,7 @@ nsNavHistory::RemoveDuplicateURIs()
     // update historyvisits so they are remapped to the retained uri
     rv = updateStatement->BindInt64Parameter(0, id);
     NS_ENSURE_SUCCESS(rv, rv);
-    rv = updateStatement->BindUTF8StringParameter(1, url);
+    rv = BindStatementURLCString(updateStatement, 1, url);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = updateStatement->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
@@ -7337,7 +7337,7 @@ nsNavHistory::RemoveDuplicateURIs()
     // remap bookmarks to the retained id
     rv = bookmarkStatement->BindInt64Parameter(0, id);
     NS_ENSURE_SUCCESS(rv, rv);
-    rv = bookmarkStatement->BindUTF8StringParameter(1, url);
+    rv = BindStatementURLCString(bookmarkStatement, 1, url);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = bookmarkStatement->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
@@ -7345,13 +7345,13 @@ nsNavHistory::RemoveDuplicateURIs()
     // remap annotations to the retained id
     rv = annoStatement->BindInt64Parameter(0, id);
     NS_ENSURE_SUCCESS(rv, rv);
-    rv = annoStatement->BindUTF8StringParameter(1, url);
+    rv = BindStatementURLCString(annoStatement, 1, url);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = annoStatement->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
     
     // remove duplicate uris from moz_places
-    rv = deleteStatement->BindUTF8StringParameter(0, url);
+    rv = BindStatementURLCString(deleteStatement, 0, url);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = deleteStatement->BindInt64Parameter(1, id);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -7528,10 +7528,6 @@ void ParseSearchTermsFromQueries(const nsCOMArray<nsNavHistoryQuery>& aQueries,
 } // anonymous namespace
 
 
-/**
- * Binds the specified URI as the parameter 'index' for the statment.
- * URIs are always bound as UTF8
- */
 nsresult
 BindStatementURI(mozIStorageStatement* statement, PRInt32 index, nsIURI* aURI)
 {
@@ -7542,12 +7538,26 @@ BindStatementURI(mozIStorageStatement* statement, PRInt32 index, nsIURI* aURI)
   nsresult rv = aURI->GetSpec(utf8URISpec);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = statement->BindUTF8StringParameter(index,
-      StringHead(utf8URISpec, URI_LENGTH_MAX));
+  rv = BindStatementURLCString(statement, index, utf8URISpec);
   NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
+
+nsresult
+BindStatementURLCString(mozIStorageStatement* statement,
+                        PRInt32 index,
+                        const nsACString& aURLString)
+{
+  NS_ASSERTION(statement, "Must have non-null statement");
+
+  nsresult rv = statement->BindUTF8StringParameter(
+    index, StringHead(aURLString, URI_LENGTH_MAX));
+  NS_ENSURE_SUCCESS(rv, rv);
+  return NS_OK;
+}
+
+
 nsresult
 nsNavHistory::UpdateFrecency(PRInt64 aPlaceId, PRBool aIsBookmarked)
 {

toolkit/components/places/src/nsNavHistory.h

@@ -701,12 +701,17 @@ protected:
 };
 
 /**
- * Shared between the places components, this function binds the given URI as
- * UTF8 to the given parameter for the statement.
+ * These utils bind a specified URI (or URL) to a statement, at the specified
+ * index.
+ * @note URIs are always bound as UTF8.
  */
-nsresult BindStatementURI(mozIStorageStatement* statement, PRInt32 index,
+nsresult BindStatementURI(mozIStorageStatement* statement,
+                          PRInt32 index,
                           nsIURI* aURI);
-
+nsresult BindStatementURLCString(mozIStorageStatement* statement,
+                                 PRInt32 index,
+                                 const nsACString& aURLString);
+                        
 #define PLACES_URI_PREFIX "place:"
 
 /* Returns true if the given URI represents a history query. */

toolkit/components/places/tests/unit/test_536081.js

@@ -0,0 +1,80 @@
+/* -*- 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 unit test code.
+ *
+ * The Initial Developer of the Original Code is the Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Marco Bonardo <mak77bonardo.net> (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 ***** */
+
+let hs = Cc["@mozilla.org/browser/nav-history-service;1"].
+         getService(Ci.nsINavHistoryService);
+let bh = hs.QueryInterface(Ci.nsIBrowserHistory);
+let db = hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
+
+const URLS = [
+  { u: "http://www.google.com/search?q=testing%3Bthis&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a",
+    s: "goog" },
+];
+
+function run_test() {
+  URLS.forEach(test_url);
+}
+
+function test_url(aURL) {
+  print("Testing url: " + aURL.u);
+  hs.addVisit(uri(aURL.u), Date.now() * 1000, null, hs.TRANSITION_TYPED, false, 0);
+  let query = hs.getNewQuery();
+  query.searchTerms = aURL.s;
+  let options = hs.getNewQueryOptions();
+  let root = hs.executeQuery(query, options).root;
+  root.containerOpen = true;
+  let cc = root.childCount;
+  do_check_eq(cc, 1);
+  print("Checking url is in the query.");
+  let node = root.getChild(0);
+  print("Found " + node.uri);
+  root.containerOpen = false;
+  bh.removePage(uri(node.uri));
+}
+
+function check_empty_table(table_name) {
+  print("Checking url has been removed.");
+  let stmt = db.createStatement("SELECT count(*) FROM " + table_name);
+  try {
+    stmt.executeStep();
+    do_check_eq(stmt.getInt32(0), 0);
+  }
+  finally {
+    stmt.finalize();
+  }
+}