Bug 817232 - Don't apply incoming deletions for special folders. r=g…

…ps, a=akeybl
kzth · Dec 5, 2012 · 8f18011 · 8f18011
1 parent 4b50c0d
commit 8f18011
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 2 deletions.
diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js
@@ -485,14 +485,21 @@ BookmarksStore.prototype = {
   },
 
   applyIncoming: function BStore_applyIncoming(record) {
-    // Don't bother with pre and post-processing for deletions.
+    let isSpecial = record.id in kSpecialIds;
+
     if (record.deleted) {
+      if (isSpecial) {
+        this._log.warn("Ignoring deletion for special record " + record.id);
+        return;
+      }
+
+      // Don't bother with pre and post-processing for deletions.
       Store.prototype.applyIncoming.call(this, record);
       return;
     }
 
     // For special folders we're only interested in child ordering.
-    if ((record.id in kSpecialIds) && record.children) {
+    if (isSpecial && record.children) {
       this._log.debug("Processing special node: " + record.id);
       // Reorder children later
       this._childrenToOrder[record.id] = record.children;
@@ -768,6 +775,11 @@ BookmarksStore.prototype = {
   },
 
   remove: function BStore_remove(record) {
+    if (kSpecialIds.isSpecialGUID(record.id)) {
+      this._log.warn("Refusing to remove special folder " + record.id);
+      return;
+    }
+
     let itemId = this.idForGUID(record.id);
     if (itemId <= 0) {
       this._log.debug("Item " + record.id + " already removed");

diff --git a/services/sync/tests/unit/test_bookmark_store.js b/services/sync/tests/unit/test_bookmark_store.js
@@ -14,6 +14,30 @@ let store = engine._store;
 let fxuri = Utils.makeURI("http://getfirefox.com/");
 let tburi = Utils.makeURI("http://getthunderbird.com/");
 
+add_test(function test_ignore_specials() {
+  _("Ensure that we can't delete bookmark roots.");
+
+  // Belt...
+  let record = new BookmarkFolder("bookmarks", "toolbar", "folder");
+  record.deleted = true;
+  do_check_neq(null, store.idForGUID("toolbar"));
+
+  store.applyIncoming(record);
+
+  // Ensure that the toolbar exists.
+  do_check_neq(null, store.idForGUID("toolbar"));
+
+  // This will fail painfully in getItemType if the deletion worked.
+  engine._buildGUIDMap();
+
+  // Braces...
+  store.remove(record);
+  do_check_neq(null, store.idForGUID("toolbar"));
+  engine._buildGUIDMap();
+
+  store.wipe();
+  run_next_test();
+});
 
 add_test(function test_bookmark_create() {
   try {