[minor fix] Places - deleting folder of livemarks - nsLivemarkService.js - 0x80070057 (NS_ERROR_ILLEGAL_VALUE)

2 files changed, 54 insertions, 40 deletions

diff --git a/browser/components/places/PlacesUIUtils.jsm b/browser/components/places/PlacesUIUtils.jsm
index b25835a71..17fa276aa 100644
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -46,43 +46,6 @@ let gFaviconLoadDataMap = new Map();
 // copied from utilityOverlay.js
 const TAB_DROP_TYPE = "application/x-moz-tabbrowser-tab";
 
-// This function isn't public both because it's synchronous and because it is
-// going to be removed in bug 1072833.
-function IsLivemark(aItemId) {
-  // Since this check may be done on each dragover event, it's worth maintaining
-  // a cache.
-  let self = IsLivemark;
-  if (!("ids" in self)) {
-    const LIVEMARK_ANNO = PlacesUtils.LMANNO_FEEDURI;
-
-    let idsVec = PlacesUtils.annotations.getItemsWithAnnotation(LIVEMARK_ANNO);
-    self.ids = new Set(idsVec);
-
-    let obs = Object.freeze({
-      QueryInterface: XPCOMUtils.generateQI(Ci.nsIAnnotationObserver),
-
-      onItemAnnotationSet(itemId, annoName) {
-        if (annoName == LIVEMARK_ANNO)
-          self.ids.add(itemId);
-      },
-
-      onItemAnnotationRemoved(itemId, annoName) {
-        // If annoName is set to an empty string, the item is gone.
-        if (annoName == LIVEMARK_ANNO || annoName == "")
-          self.ids.delete(itemId);
-      },
-
-      onPageAnnotationSet() { },
-      onPageAnnotationRemoved() { },
-    });
-    PlacesUtils.annotations.addObserver(obs);
-    PlacesUtils.registerShutdownFunction(() => {
-      PlacesUtils.annotations.removeObserver(obs);
-    });
-  }
-  return self.ids.has(aItemId);
-}
-
 let InternalFaviconLoader = {
   /**
    * This gets called for every inner window that is destroyed.
@@ -509,6 +472,52 @@ this.PlacesUIUtils = {
   },
 
   /**
+   * Test if a bookmark item = a live bookmark item.
+   *
+   * @param aItemId
+   *        item identifier
+   * @return true if a live bookmark item, false otherwise.
+   *
+   * @note Maybe this should be removed later, see bug 1072833.
+   */
+  _isLivemark:
+  function PUIU__isLivemark(aItemId)
+  {
+    // Since this check may be done on each dragover event, it's worth maintaining
+    // a cache.
+    let self = PUIU__isLivemark;
+    if (!("ids" in self)) {
+      const LIVEMARK_ANNO = PlacesUtils.LMANNO_FEEDURI;
+
+      let idsVec = PlacesUtils.annotations.getItemsWithAnnotation(LIVEMARK_ANNO);
+      self.ids = new Set(idsVec);
+
+      let obs = Object.freeze({
+        QueryInterface: XPCOMUtils.generateQI(Ci.nsIAnnotationObserver),
+
+        onItemAnnotationSet(itemId, annoName) {
+          if (annoName == LIVEMARK_ANNO)
+            self.ids.add(itemId);
+        },
+
+        onItemAnnotationRemoved(itemId, annoName) {
+          // If annoName is set to an empty string, the item is gone.
+          if (annoName == LIVEMARK_ANNO || annoName == "")
+            self.ids.delete(itemId);
+        },
+
+        onPageAnnotationSet() { },
+        onPageAnnotationRemoved() { },
+      });
+      PlacesUtils.annotations.addObserver(obs);
+      PlacesUtils.registerShutdownFunction(() => {
+        PlacesUtils.annotations.removeObserver(obs);
+      });
+    }
+    return self.ids.has(aItemId);
+  },
+
+  /**
    * Constructs a Transaction for the drop or paste of a blob of data into
    * a container.
    * @param   data
@@ -857,7 +866,7 @@ this.PlacesUIUtils = {
       throw new Error("invalid value for aNodeOrItemId");
     }
 
-    if (itemId == PlacesUtils.placesRootId || IsLivemark(itemId))
+    if (itemId == PlacesUtils.placesRootId || this._isLivemark(itemId))
       return true;
 
     // leftPaneFolderId, and as a result, allBookmarksFolderId, is a lazy getter
diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm
index 4b7bcb82a..fc303ca8a 100644
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -53,6 +53,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
                                   "resource://gre/modules/AsyncShutdown.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesSyncUtils",
                                   "resource://gre/modules/PlacesSyncUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesUIUtils",
+                                  "resource:///modules/PlacesUIUtils.jsm");
 
 // The minimum amount of transactions before starting a batch. Usually we do
 // do incremental updates, a batch will cause views to completely
@@ -3248,8 +3250,11 @@ PlacesRemoveItemTransaction.prototype = {
     let contents =
       PlacesUtils.getFolderContents(this.item.id, false, false).root;
     for (let i = 0; i < contents.childCount; ++i) {
-      let txn = new PlacesRemoveItemTransaction(contents.getChild(i).itemId);
-      transactions.push(txn);
+      let childId = contents.getChild(i).itemId;
+      if (!PlacesUIUtils._isLivemark(childId)) {
+        let txn = new PlacesRemoveItemTransaction(childId);
+        transactions.push(txn);
+      }
     }
     contents.containerOpen = false;
     // Reverse transactions to preserve parent-child relationship.