Bug 610501: better rewriting of tag search smart bookmarks. r=philiKON

services/sync/modules/engines/bookmarks.js

@@ -487,6 +487,59 @@ BookmarksStore.prototype = {
     return this.idForGUID(id, true) > 0;
   },
   
+  /*
+   * If the record is a tag query, rewrite it to refer to the local tag ID.
+   * 
+   * Otherwise, just return.
+   */
+  preprocessTagQuery: function preprocessTagQuery(record) {
+    if (record.type != "query" ||
+        record.bmkUri == null ||
+        record.folderName == null)
+      return;
+    
+    // Yes, this works without chopping off the "place:" prefix.
+    let uri           = record.bmkUri
+    let queriesRef    = {};
+    let queryCountRef = {};
+    let optionsRef    = {};
+    Svc.History.queryStringToQueries(uri, queriesRef, queryCountRef, optionsRef);
+    
+    // We only process tag URIs.
+    if (optionsRef.value.resultType != optionsRef.value.RESULTS_AS_TAG_CONTENTS)
+      return;
+    
+    // Tag something to ensure that the tag exists.
+    let tag = record.folderName;
+    let dummyURI = Utils.makeURI("about:weave#BStore_preprocess");
+    this._ts.tagURI(dummyURI, [tag]);
+
+    // Look for the id of the tag, which might just have been added.
+    let tags = this._getNode(this._bms.tagsFolder);
+    if (!(tags instanceof Ci.nsINavHistoryQueryResultNode)) {
+      this._log.debug("tags isn't an nsINavHistoryQueryResultNode; aborting.");
+      return;
+    }
+
+    tags.containerOpen = true;
+    for (let i = 0; i < tags.childCount; i++) {
+      let child = tags.getChild(i);
+      if (child.title == tag) {
+        // Found the tag, so fix up the query to use the right id.
+        this._log.debug("Tag query folder: " + tag + " = " + child.itemId);
+        
+        this._log.trace("Replacing folders in: " + uri);
+        for each (let q in queriesRef.value)
+          q.setFolders([child.itemId], 1);
+        
+        record.bmkUri = Svc.History.queriesToQueryString(queriesRef.value,
+                                                         queryCountRef.value,
+                                                         optionsRef.value);
+        return;
+      }
+    }
+  },
+  
   applyIncoming: function BStore_applyIncoming(record) {
     // Don't bother with pre and post-processing for deletions.
     if (record.deleted) {
@@ -502,37 +555,8 @@ BookmarksStore.prototype = {
       return;
     }
 
-    // Preprocess the record before doing the normal apply
-    switch (record.type) {
-      case "query": {
-        // Convert the query uri if necessary
-        if (record.bmkUri == null || record.folderName == null)
-          break;
-
-        // Tag something so that the tag exists
-        let tag = record.folderName;
-        let dummyURI = Utils.makeURI("about:weave#BStore_preprocess");
-        this._ts.tagURI(dummyURI, [tag]);
-
-        // Look for the id of the tag (that might have just been added)
-        let tags = this._getNode(this._bms.tagsFolder);
-        if (!(tags instanceof Ci.nsINavHistoryQueryResultNode))
-          break;
-
-        tags.containerOpen = true;
-        for (let i = 0; i < tags.childCount; i++) {
-          let child = tags.getChild(i);
-          // Found the tag, so fix up the query to use the right id
-          if (child.title == tag) {
-            this._log.debug("query folder: " + tag + " = " + child.itemId);
-            record.bmkUri = record.bmkUri.replace(/([:&]folder=)\d+/, "$1" +
-              child.itemId);
-            break;
-          }
-        }
-        break;
-      }
-    }
+    // Preprocess the record before doing the normal apply.
+    this.preprocessTagQuery(record);
 
     // Figure out the local id of the parent GUID if available
     let parentGUID = record.parentid;

services/sync/tests/unit/test_bookmark_places_query_rewriting.js

@@ -0,0 +1,45 @@
+_("Rewrite place: URIs.");
+Cu.import("resource://services-sync/engines/bookmarks.js");
+Cu.import("resource://services-sync/util.js");
+
+let engine = new BookmarksEngine();
+let store = engine._store;
+
+function run_test() {
+  initTestLogging("Trace");
+  Log4Moz.repository.getLogger("Engine.Bookmarks").level = Log4Moz.Level.Trace;
+  Log4Moz.repository.getLogger("Store.Bookmarks").level = Log4Moz.Level.Trace;
+
+  let tagRecord = new BookmarkQuery("bookmarks", "abcdefabcdef");
+  let uri = "place:folder=499&type=7&queryType=1";
+  tagRecord.queryId = "MagicTags";
+  tagRecord.parentName = "Bookmarks Toolbar";
+  tagRecord.bmkUri = uri;
+  tagRecord.title = "tagtag";
+  tagRecord.folderName = "bar";
+
+  _("Type: " + tagRecord.type);
+  _("Folder name: " + tagRecord.folderName);
+  store.preprocessTagQuery(tagRecord);
+  
+  _("Verify that the URI has been rewritten.");
+  do_check_neq(tagRecord.bmkUri, uri);
+  
+  let tags = store._getNode(store._bms.tagsFolder);
+  tags.containerOpen = true;
+  let tagID;
+  for (let i = 0; i < tags.childCount; ++i) {
+    let child = tags.getChild(i);
+    if (child.title == "bar")
+      tagID = child.itemId;
+  }
+      
+  _("Tag ID: " + tagID);
+  do_check_eq(tagRecord.bmkUri, uri.replace("499", tagID));
+  
+  _("... but not if the type is wrong.");
+  let wrongTypeURI = "place:folder=499&type=2&queryType=1";
+  tagRecord.bmkUri = wrongTypeURI;
+  store.preprocessTagQuery(tagRecord);
+  do_check_eq(tagRecord.bmkUri, wrongTypeURI);
+}