Bug 1194568 - Renaming live bookmark while adding it renames the wrong one. r=ttaubert

--HG--
extra : commitid : Fom0hKHZmzq

browser/components/places/content/bookmarkProperties.js

@@ -583,48 +583,44 @@ var BookmarkPropertiesPanel = {
                                              childItemsTransactions);
   },
 
-  /**
-   * Returns a transaction for creating a new live-bookmark item representing
-   * the various fields and opening arguments of the dialog.
-   */
-  _getCreateNewLivemarkTransaction:
-  function BPP__getCreateNewLivemarkTransaction(aContainer, aIndex) {
-    return new PlacesCreateLivemarkTransaction(this._feedURI, this._siteURI,
-                                               this._title,
-                                               aContainer, aIndex);
-  },
-
-  _createNewItem: function BPP__getCreateItemTransaction() {
-    var [container, index] = this._getInsertionPointDetails();
-    var txn;
-
+  _createNewItem: Task.async(function* () {
+    let [container, index] = this._getInsertionPointDetails();
+    let txn;
     switch (this._itemType) {
       case BOOKMARK_FOLDER:
         txn = this._getCreateNewFolderTransaction(container, index);
         break;
       case LIVEMARK_CONTAINER:
-        txn = this._getCreateNewLivemarkTransaction(container, index);
+        txn = new PlacesCreateLivemarkTransaction(this._feedURI, this._siteURI,
+                                                  this._title, container, index);
         break;
       default: // BOOKMARK_ITEM
         txn = this._getCreateNewBookmarkTransaction(container, index);
     }
 
     PlacesUtils.transactionManager.doTransaction(txn);
-    this._itemId = PlacesUtils.bookmarks.getIdForItemAt(container, index);
+    // This is a temporary hack until we use PlacesTransactions.jsm
+    if (txn._promise) {
+      yield txn._promise;
+    }
+
+    let folderGuid = yield PlacesUtils.promiseItemGuid(container);
+    let bm = yield PlacesUtils.bookmarks.fetch({
+      parentGuid: folderGuid,
+      index: PlacesUtils.bookmarks.DEFAULT_INDEX
+    });
+    this._itemId = yield PlacesUtils.promiseItemId(bm.guid);
 
     return Object.freeze({
       itemId: this._itemId,
-      get bookmarkGuid() {
-        throw new Error("Node-like bookmarkGuid getter called even though " +
-                        "async transactions are disabled");
-      },
+      bookmarkGuid: bm.guid,
       title: this._title,
       uri: this._uri ? this._uri.spec : "",
       type: this._itemType == BOOKMARK_ITEM ?
               Ci.nsINavHistoryResultNode.RESULT_TYPE_URI :
               Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER
     });
-  },
+  }),
 
   _promiseNewItem: Task.async(function* () {
     if (!PlacesUIUtils.useAsyncTransactions)

browser/components/places/tests/browser/browser.ini

@@ -25,6 +25,7 @@ support-files =
   pageopeningwindow.html
 [browser_bookmarkProperties_addFolderDefaultButton.js]
 [browser_bookmarkProperties_addKeywordForThisSearch.js]
+[browser_bookmarkProperties_addLivemark.js]
 [browser_bookmarkProperties_editTagContainer.js]
 [browser_bookmarkProperties_readOnlyRoot.js]
 [browser_bookmarksProperties.js]

browser/components/places/tests/browser/browser_bookmarkProperties_addLivemark.js

@@ -0,0 +1,39 @@
+"use strict"
+
+add_task(function* () {
+  info("Add a live bookmark editing its data");
+
+  yield withSidebarTree("bookmarks", function* (tree) {
+    let itemId = PlacesUIUtils.leftPaneQueries["UnfiledBookmarks"];
+    tree.selectItems([itemId]);
+
+    yield withBookmarksDialog(
+      true,
+      function openDialog() {
+        PlacesCommandHook.addLiveBookmark("http://livemark.com/",
+                                          "livemark", "description");
+      },
+      function* test(dialogWin) {
+        let promiseTitleChangeNotification = promiseBookmarksNotification(
+          "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "modified");
+
+        fillBookmarkTextField("editBMPanel_namePicker", "modified", dialogWin);
+
+        yield promiseTitleChangeNotification;
+
+        let bookmark = yield PlacesUtils.bookmarks.fetch({
+          parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+          index: PlacesUtils.bookmarks.DEFAULT_INDEX
+        });
+
+        is(bookmark.title, "modified", "folder name has been edited");
+
+        let livemark = yield PlacesUtils.livemarks.getLivemark({
+          guid: bookmark.guid
+        });
+        is(livemark.feedURI.spec, "http://livemark.com/", "livemark has the correct url");
+        is(livemark.title, "modified", "livemark has the correct title");
+      }
+    );
+  });
+});

browser/components/places/tests/browser/head.js

@@ -297,7 +297,7 @@ let withBookmarksDialog = Task.async(function* (autoCancel, openFn, taskFn) {
         win.addEventListener("load", function load() {
           win.removeEventListener("load", load);
           ok(win.location.href.startsWith("chrome://browser/content/places/bookmarkProperties"),
-             "The bookmark properties dialog is ready");
+             "The bookmark properties dialog is open");
           // This is needed for the overlay.
           waitForFocus(() => {
             resolve(win);
@@ -318,7 +318,9 @@ let withBookmarksDialog = Task.async(function* (autoCancel, openFn, taskFn) {
   let dialogWin = yield dialogPromise;
 
   // Ensure overlay is loaded
-  ok(dialogWin.gEditItemOverlay.initialized, "EditItemOverlay is initialized");
+  info("waiting for the overlay to be loaded");
+  yield waitForCondition(() => dialogWin.gEditItemOverlay.initialized,
+                         "EditItemOverlay should be initialized");
 
   info("withBookmarksDialog: executing the task");
   try {

toolkit/components/places/PlacesUtils.jsm

@@ -2911,7 +2911,7 @@ PlacesCreateLivemarkTransaction.prototype = {
 
   doTransaction: function CLTXN_doTransaction()
   {
-    PlacesUtils.livemarks.addLivemark(
+    this._promise = PlacesUtils.livemarks.addLivemark(
       { title: this.item.title
       , feedURI: this.item.feedURI
       , parentId: this.item.parentId
@@ -2930,7 +2930,7 @@ PlacesCreateLivemarkTransaction.prototype = {
   {
     // The getLivemark callback may fail, but it is used just to serialize,
     // so it doesn't matter.
-    PlacesUtils.livemarks.getLivemark({ id: this.item.id })
+    this._promise = PlacesUtils.livemarks.getLivemark({ id: this.item.id })
       .then(null, null).then( () => {
         PlacesUtils.bookmarks.removeItem(this.item.id);
       });