Bug 1167915 - "Add a Keyword for this Search" does not work anymore on POST forms. r=ttaubert
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 03 Jun 2015 21:12:48 +0200
changeset 246919 b8ae983d49371e9da7280e1658f02b788682ab3a
parent 246918 7969c82a42dfacc6b6a1b19d6e10f7b9e1922ea8
child 247054 cd451afd6400e9ca96fd9a6062bda0bff19f6af8
push id13335
push usermak77@bonardo.net
push dateWed, 03 Jun 2015 19:13:12 +0000
treeherderfx-team@b8ae983d4937 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert
bugs1167915
milestone41.0a1
Bug 1167915 - "Add a Keyword for this Search" does not work anymore on POST forms. r=ttaubert
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/bookmarkProperties.js
browser/components/places/content/editBookmarkOverlay.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
browser/components/places/tests/browser/head.js
browser/components/places/tests/browser/keyword_form.html
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_placesTxn.js
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -1466,19 +1466,16 @@ XPCOMUtils.defineLazyGetter(PlacesUIUtil
       new PlacesSetItemAnnotationTransaction(aItemId, aAnnotationObject),
 
     setPageAnnotation: function(aURI, aAnnotationObject)
       new PlacesSetPageAnnotationTransaction(aURI, aAnnotationObject),
 
     editBookmarkKeyword: function(aItemId, aNewKeyword)
       new PlacesEditBookmarkKeywordTransaction(aItemId, aNewKeyword),
 
-    editBookmarkPostData: function(aItemId, aPostData)
-      new PlacesEditBookmarkPostDataTransaction(aItemId, aPostData),
-
     editLivemarkSiteURI: function(aLivemarkId, aSiteURI)
       new PlacesEditLivemarkSiteURITransaction(aLivemarkId, aSiteURI),
 
     editLivemarkFeedURI: function(aLivemarkId, aFeedURI)
       new PlacesEditLivemarkFeedURITransaction(aLivemarkId, aFeedURI),
 
     editItemDateAdded: function(aItemId, aNewDateAdded)
       new PlacesEditItemDateAddedTransaction(aItemId, aNewDateAdded),
--- a/browser/components/places/content/bookmarkProperties.js
+++ b/browser/components/places/content/bookmarkProperties.js
@@ -278,17 +278,18 @@ var BookmarkPropertiesPanel = {
         gEditItemOverlay.initPanel({ node: this._node
                                    , hiddenRows: this._hiddenRows });
         acceptButton.disabled = gEditItemOverlay.readOnly;
         break;
       case ACTION_ADD:
         this._node = yield this._promiseNewItem();
         // Edit the new item
         gEditItemOverlay.initPanel({ node: this._node
-                                   , hiddenRows: this._hiddenRows });
+                                   , hiddenRows: this._hiddenRows
+                                   , postData: this._postData });
 
         // Empty location field if the uri is about:blank, this way inserting a new
         // url will be easier for the user, Accept button will be automatically
         // disabled by the input listener until the user fills the field.
         let locationField = this._element("locationField");
         if (locationField.value == "about:blank")
           locationField.value = "";
 
@@ -520,32 +521,28 @@ var BookmarkPropertiesPanel = {
 
     if (this._loadInSidebar) {
       let annoObj = { name   : PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO,
                       value  : true };
       let setLoadTxn = new PlacesSetItemAnnotationTransaction(-1, annoObj);
       childTransactions.push(setLoadTxn);
     }
 
-    if (this._postData) {
-      let postDataTxn = new PlacesEditBookmarkPostDataTransaction(-1, this._postData);
-      childTransactions.push(postDataTxn);
-    }
-
     //XXX TODO: this should be in a transaction!
     if (this._charSet && !PrivateBrowsingUtils.isWindowPrivate(window))
       PlacesUtils.setCharsetForURI(this._uri, this._charSet);
 
     let createTxn = new PlacesCreateBookmarkTransaction(this._uri,
                                                         aContainer,
                                                         aIndex,
                                                         this._title,
                                                         this._keyword,
                                                         annotations,
-                                                        childTransactions);
+                                                        childTransactions,
+                                                        this._postData);
 
     return new PlacesAggregatedTransaction(this._getDialogTitle(),
                                            [createTxn]);
   },
 
   /**
    * Returns a childItems-transactions array representing the URIList with
    * which the dialog has been opened.
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -37,29 +37,30 @@ let gEditItemOverlay = {
     let isURI = node && PlacesUtils.nodeIsURI(node);
     let uri = isURI ? NetUtil.newURI(node.uri) : null;
     let title = node ? node.title : null;
     let isBookmark = isItem && isURI;
     let bulkTagging = !node;
     let uris = bulkTagging ? aInitInfo.uris : null;
     let visibleRows = new Set();
     let isParentReadOnly = false;
+    let postData = aInitInfo.postData;
     if (node && "parent" in node) {
       let parent = node.parent;
       if (parent) {
         isParentReadOnly = !PlacesUtils.nodeIsFolder(parent) ||
                             PlacesUIUtils.isContentsReadOnly(parent);
       }
     }
 
     return this._paneInfo = { itemId, itemGuid, isItem,
                               isURI, uri, title,
                               isBookmark, isFolderShortcut, isParentReadOnly,
                               bulkTagging, uris,
-                              visibleRows };
+                              visibleRows, postData };
   },
 
   get initialized() {
     return this._paneInfo != null;
   },
 
   // Backwards-compatibility getters
   get itemId() {
@@ -591,17 +592,17 @@ let gEditItemOverlay = {
 
   onKeywordFieldChange() {
     if (this.readOnly || !this._paneInfo.isBookmark)
       return;
 
     let itemId = this._paneInfo.itemId;
     let newKeyword = this._keywordField.value;
     if (!PlacesUIUtils.useAsyncTransactions) {
-      let txn = new PlacesEditBookmarkKeywordTransaction(itemId, newKeyword);
+      let txn = new PlacesEditBookmarkKeywordTransaction(itemId, newKeyword, this._paneInfo.postData);
       PlacesUtils.transactionManager.doTransaction(txn);
       return;
     }
     let guid = this._paneInfo.itemGuid;
     PlacesTransactions.EditKeyword({ guid, keyword: newKeyword })
                       .transact().catch(Components.utils.reportError);
   },
 
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -5,47 +5,45 @@
 [DEFAULT]
 skip-if = buildapp == "mulet"
 support-files =
   head.js
   framedPage.html
   frameLeft.html
   frameRight.html
   sidebarpanels_click_test_page.html
+  keyword_form.html
 
 [browser_0_library_left_pane_migration.js]
-[browser_library_left_pane_fixnames.js]
-[browser_425884.js]
-[browser_475045.js]
+[browser_410196_paste_into_tags.js]
+[browser_416459_cut.js]
 [browser_423515.js]
-[browser_410196_paste_into_tags.js]
-[browser_sort_in_library.js]
-[browser_library_open_leak.js]
-[browser_library_panel_leak.js]
-[browser_library_search.js]
-[browser_history_sidebar_search.js]
-skip-if = e10s && (os == 'linux' || os == 'mac') # Bug 1116457
+[browser_425884.js]
+[browser_435851_copy_query.js]
+[browser_475045.js]
+[browser_555547.js]
+[browser_bookmarkProperties_addKeywordForThisSearch.js]
 [browser_bookmarksProperties.js]
-
-[browser_forgetthissite_single.js]
-[browser_library_commands.js]
 [browser_drag_bookmarks_on_toolbar.js]
 skip-if = e10s # Bug ?????? - test fails - "Number of dragged items should be the same. - Got 0, expected 1"
+[browser_forgetthissite_single.js]
+[browser_history_sidebar_search.js]
+skip-if = e10s && (os == 'linux' || os == 'mac') # Bug 1116457
+[browser_library_batch_delete.js]
+[browser_library_commands.js]
+[browser_library_downloads.js]
+[browser_library_infoBox.js]
+[browser_library_left_pane_fixnames.js]
+[browser_library_left_pane_select_hierarchy.js]
 [browser_library_middleclick.js]
+[browser_library_open_leak.js]
+[browser_library_openFlatContainer.js]
+[browser_library_panel_leak.js]
+[browser_library_search.js]
 [browser_library_views_liveupdate.js]
-[browser_views_liveupdate.js]
-
-[browser_sidebarpanels_click.js]
-# temporarily disabled for breaking the treeview - bug 658744
-skip-if = true
-
-[browser_library_infoBox.js]
 [browser_markPageAsFollowedLink.js]
 skip-if = e10s # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly (test does EventUtils.sendMouseEvent...)
+[browser_sidebarpanels_click.js]
+skip-if = true # temporarily disabled for breaking the treeview - bug 658744
+[browser_sort_in_library.js]
 [browser_toolbar_migration.js]
-[browser_library_batch_delete.js]
-[browser_555547.js]
-[browser_416459_cut.js]
-[browser_library_downloads.js]
-[browser_library_left_pane_select_hierarchy.js]
-[browser_435851_copy_query.js]
 [browser_toolbarbutton_menu_context.js]
-[browser_library_openFlatContainer.js]
+[browser_views_liveupdate.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
@@ -0,0 +1,57 @@
+"use strict"
+
+const TEST_URL = "http://mochi.test:8888/browser/browser/components/places/tests/browser/keyword_form.html";
+
+add_task(function* () {
+  yield BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: TEST_URL,
+  }, function* (browser) {
+    // We must wait for the context menu code to build metadata.
+    yield openContextMenuForContentSelector(browser, 'form > input[name="search"]');
+
+    yield withBookmarksDialog(function*() {
+      AddKeywordForSearchField();
+    }, function* (dialogWin) {
+      let acceptBtn = dialogWin.document.documentElement.getButton("accept");
+      ok(acceptBtn.disabled, "Accept button is disabled");
+
+      let promiseKeywordNotification = promiseBookmarksNotification(
+        "onItemChanged", (itemId, prop, isAnno, val) => prop == "keyword" && val =="kw");
+
+      fillBookmarkTextField("editBMPanel_keywordField", "kw", dialogWin);
+
+      ok(!acceptBtn.disabled, "Accept button is enabled");
+
+      // The dialog is instant apply.
+      yield promiseKeywordNotification;
+
+      // After the notification, the keywords cache will update asynchronously.
+      info("Check the keyword entry has been created");
+      let entry;
+      yield waitForCondition(function* () {
+        entry = yield PlacesUtils.keywords.fetch("kw");
+        return !!entry;
+      },"Unable to find the expected keyword");
+      is(entry.keyword, "kw", "keyword is correct");
+      is(entry.url.href, TEST_URL, "URL is correct");
+      is(entry.postData, "accenti%3D%E0%E8%EC%F2%F9&search%3D%25s", "POST data is correct");
+
+      info("Check the charset has been saved");
+      let charset = yield PlacesUtils.getCharsetForURI(NetUtil.newURI(TEST_URL));
+      is(charset, "windows-1252", "charset is correct");
+
+      // Now check getShortcutOrURI.
+      let data = yield getShortcutOrURIAndPostData("kw test");
+      is(getPostDataString(data.postData), "accenti=\u00E0\u00E8\u00EC\u00F2\u00F9&search=test", "getShortcutOrURI POST data is correct");
+      is(data.url, TEST_URL, "getShortcutOrURI URL is correct");
+    });
+  });
+});
+
+function getPostDataString(stream) {
+  let sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+  sis.init(stream);
+  return sis.read(stream.available()).split("\n").pop();
+}
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -157,28 +157,31 @@ function promiseIsURIVisited(aURI) {
   PlacesUtils.asyncHistory.isURIVisited(aURI, function(aURI, aIsVisited) {
     deferred.resolve(aIsVisited);
   });
 
   return deferred.promise;
 }
 
 function promiseBookmarksNotification(notification, conditionFn) {
-  info(`Waiting for ${notification}`);
+  info(`promiseBookmarksNotification: waiting for ${notification}`);
   return new Promise((resolve, reject) => {
     let proxifiedObserver = new Proxy({}, {
       get: (target, name) => {
         if (name == "QueryInterface")
           return XPCOMUtils.generateQI([ Ci.nsINavBookmarkObserver ]);
+        info(`promiseBookmarksNotification: got ${name} notification`);
         if (name == notification)
           return () => {
             if (conditionFn.apply(this, arguments)) {
               clearTimeout(timeout);
               PlacesUtils.bookmarks.removeObserver(proxifiedObserver, false);
               executeSoon(resolve);
+            } else {
+              info(`promiseBookmarksNotification: skip cause condition doesn't apply to ${JSON.stringify(arguments)}`);
             }
           }
         return () => {};
       }
     });
     PlacesUtils.bookmarks.addObserver(proxifiedObserver, false);
     let timeout = setTimeout(() => {
       PlacesUtils.bookmarks.removeObserver(proxifiedObserver, false);
@@ -273,8 +276,136 @@ function promiseSetToolbarVisibility(aTo
 function isToolbarVisible(aToolbar) {
   let hidingAttribute = aToolbar.getAttribute("type") == "menubar"
                         ? "autohide"
                         : "collapsed";
   let hidingValue = aToolbar.getAttribute(hidingAttribute).toLowerCase();
   // Check for both collapsed="true" and collapsed="collapsed"
   return hidingValue !== "true" && hidingValue !== hidingAttribute;
 }
+
+/**
+ * Executes a task after opening the bookmarks dialog, then cancels the dialog.
+ *
+ * @param openFn
+ *        generator function causing the dialog to open
+ * @param task
+ *        the task to execute once the dialog is open
+ */
+let withBookmarksDialog = Task.async(function* (openFn, taskFn) {
+  let dialogPromise = new Promise(resolve => {
+    Services.ww.registerNotification(function winObserver(subject, topic, data) {
+      if (topic != "domwindowopened")
+        return;
+      let win = subject.QueryInterface(Ci.nsIDOMWindow);
+      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");
+        Services.ww.unregisterNotification(winObserver);
+        // This is needed for the overlay.
+        waitForFocus(() => {
+          resolve(win);
+        }, win);
+      });
+    });
+  });
+
+  info("withBookmarksDialog: opening the dialog");
+  yield openFn();
+  info("withBookmarksDialog: waiting for the dialog");
+  let dialogWin = yield dialogPromise;
+
+  // Ensure overlay is loaded
+  ok(dialogWin.gEditItemOverlay.initialized, "EditItemOverlay is initialized");
+
+  info("withBookmarksDialog: executing the task");
+  try {
+    yield taskFn(dialogWin);
+  } finally {
+    info("withBookmarksDialog: canceling the dialog");
+    dialogWin.document.documentElement.cancelDialog();
+  }
+});
+
+/**
+ * Opens the contextual menu on the element pointed by the given selector.
+ *
+ * @param selector
+ *        Valid selector syntax
+ * @return the target DOM node.
+ */
+let openContextMenuForContentSelector = Task.async(function* (browser, selector) {
+  info("wait for the context menu");
+  let contextPromise = BrowserTestUtils.waitForEvent(document.getElementById("contentAreaContextMenu"),
+                                                     "popupshown");
+  yield ContentTask.spawn(browser, { selector }, function* (args) {
+    let doc = content.document;
+    let elt = doc.querySelector(args.selector)
+    dump(`openContextMenuForContentSelector: found ${elt}\n`);
+
+    /* Open context menu so chrome can access the element */
+    const domWindowUtils =
+      content.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+             .getInterface(Components.interfaces.nsIDOMWindowUtils);
+    let rect = elt.getBoundingClientRect();
+    let left = rect.left + rect.width / 2;
+    let top = rect.top + rect.height / 2;
+    domWindowUtils.sendMouseEvent("contextmenu", left, top, 2,
+                                  1, 0, false, 0, 0, true);
+  });
+  yield contextPromise;
+
+  return gContextMenuContentData.popupNode;
+});
+
+/**
+ * Waits for a specified condition to happen.
+ *
+ * @param conditionFn
+ *        a Function or a generator function, returning a boolean for whether
+ *        the condition is fulfilled.
+ * @param errorMsg
+ *        Error message to use if the condition has not been satisfied after a
+ *        meaningful amount of tries.
+ */
+let waitForCondition = Task.async(function* (conditionFn, errorMsg) {
+  for (let tries = 0; tries < 100; ++tries) {
+    if ((yield conditionFn()))
+      return;
+    yield new Promise(resolve => {
+      if (!waitForCondition._timers) {
+        waitForCondition._timers = new Set();
+        registerCleanupFunction(() => {
+          is(waitForCondition._timers.size, 0, "All the wait timers have been removed");
+          delete waitForCondition._timers;
+        });
+      }
+      let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+      waitForCondition._timers.add(timer);
+      timer.init(() => {
+        waitForCondition._timers.delete(timer);
+        resolve();
+      }, 100, Ci.nsITimer.TYPE_ONE_SHOT);
+    });
+  }
+  ok(false, errorMsg);
+});
+
+/**
+ * Fills a bookmarks dialog text field ensuring to cause expected edit events.
+ *
+ * @param id
+ *        id of the text field
+ * @param text
+ *        text to fill in
+ * @param win
+ *        dialog window
+ */
+function fillBookmarkTextField(id, text, win) {
+  let elt = win.document.getElementById(id);
+  elt.focus();
+  elt.select();
+  for (let c of text.split("")) {
+    EventUtils.synthesizeKey(c, {}, win);
+  }
+  elt.blur();
+}
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/keyword_form.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML>
+
+<html lang="en">
+<head>
+  <meta http-equiv="Content-Type" content="text/html;charset=windows-1252">
+</head>
+<body>
+  <form method="POST" action="keyword_form.html">
+    <input type="hidden" name="accenti" value="אטלעש">
+    <input type="text" name="search">
+  </form>
+</body>
+</html>
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2753,46 +2753,53 @@ PlacesCreateFolderTransaction.prototype 
  *        array of annotations to set for the new bookmark
  * @param [optional] aChildTransactions
  *        child transactions to commit after creating the bookmark. Prefer
  *        using any of the arguments above if possible. In general, a child
  *        transations should be used only if the change it does has to be
  *        reverted manually when removing the bookmark item.
  *        a child transaction must support setting its bookmark-item
  *        identifier via an "id" js setter.
+ * @param [optional] aPostData
+ *        keyword's POST data, if available.
  *
  * @return nsITransaction object
  */
 this.PlacesCreateBookmarkTransaction =
  function PlacesCreateBookmarkTransaction(aURI, aParentId, aIndex, aTitle,
                                           aKeyword, aAnnotations,
-                                          aChildTransactions)
+                                          aChildTransactions, aPostData)
 {
   this.item = new TransactionItemCache();
   this.item.uri = aURI;
   this.item.parentId = aParentId;
   this.item.index = aIndex;
   this.item.title = aTitle;
   this.item.keyword = aKeyword;
+  this.item.postData = aPostData;
   this.item.annotations = aAnnotations;
   this.childTransactions = aChildTransactions;
 }
 
 PlacesCreateBookmarkTransaction.prototype = {
   __proto__: BaseTransaction.prototype,
 
   doTransaction: function CITXN_doTransaction()
   {
     this.item.id = PlacesUtils.bookmarks.insertBookmark(this.item.parentId,
                                                         this.item.uri,
                                                         this.item.index,
                                                         this.item.title);
     if (this.item.keyword) {
       PlacesUtils.bookmarks.setKeywordForBookmark(this.item.id,
                                                   this.item.keyword);
+      if (this.item.postData) {
+        PlacesUtils.setPostDataForBookmark(this.item.id,
+                                           this.item.postData);
+      }
     }
     if (this.item.annotations && this.item.annotations.length > 0)
       PlacesUtils.setAnnotationsForItem(this.item.id, this.item.annotations);
  
     if (this.childTransactions && this.childTransactions.length > 0) {
       // Set the new item id into child transactions.
       for (let i = 0; i < this.childTransactions.length; ++i) {
         this.childTransactions[i].item.id = this.item.id;
@@ -3069,16 +3076,18 @@ this.PlacesRemoveItemTransaction =
     // Remove this folder itself.
     let txn = PlacesUtils.bookmarks.getRemoveFolderTransaction(this.item.id);
     this.childTransactions.push(txn);
   }
   else if (this.item.itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK) {
     this.item.uri = PlacesUtils.bookmarks.getBookmarkURI(this.item.id);
     this.item.keyword =
       PlacesUtils.bookmarks.getKeywordForBookmark(this.item.id);
+    if (this.item.keyword)
+      this.item.postData = PlacesUtils.getPostDataForBookmark(this.item.id);
   }
 
   if (this.item.itemType != Ci.nsINavBookmarksService.TYPE_SEPARATOR)
     this.item.title = PlacesUtils.bookmarks.getItemTitle(this.item.id);
 
   this.item.parentId = PlacesUtils.bookmarks.getFolderIdForItem(this.item.id);
   this.item.annotations = PlacesUtils.getAnnotationsForItem(this.item.id);
   this.item.dateAdded = PlacesUtils.bookmarks.getItemDateAdded(this.item.id);
@@ -3120,16 +3129,19 @@ PlacesRemoveItemTransaction.prototype = 
                                                           this.item.uri,
                                                           this.item.index,
                                                           this.item.title);
       if (this.item.tags && this.item.tags.length > 0)
         PlacesUtils.tagging.tagURI(this.item.uri, this.item.tags);
       if (this.item.keyword) {
         PlacesUtils.bookmarks.setKeywordForBookmark(this.item.id,
                                                     this.item.keyword);
+        if (this.item.postData) {
+          PlacesUtils.bookmarks.setPostDataForBookmark(this.item.id);
+        }
       }
     }
     else if (this.item.itemType == Ci.nsINavBookmarksService.TYPE_FOLDER) {
       let txn = new PlacesAggregatedTransaction("Remove item childTxn",
                                                 this.childTransactions);
       txn.undoTransaction();
     }
     else { // TYPE_SEPARATOR
@@ -3368,40 +3380,52 @@ PlacesSetPageAnnotationTransaction.proto
 
 /**
  * Transaction for editing a bookmark's keyword.
  *
  * @param aItemId
  *        id of the bookmark to edit
  * @param aNewKeyword
  *        new keyword for the bookmark
+ * @param aNewPostData [optional]
+ *        new keyword's POST data, if available
  *
  * @return nsITransaction object
  */
 this.PlacesEditBookmarkKeywordTransaction =
- function PlacesEditBookmarkKeywordTransaction(aItemId, aNewKeyword)
+ function PlacesEditBookmarkKeywordTransaction(aItemId, aNewKeyword, aNewPostData)
 {
   this.item = new TransactionItemCache();
   this.item.id = aItemId;
   this.new = new TransactionItemCache();
   this.new.keyword = aNewKeyword;
+  this.new.postData = aNewPostData
 }
 
 PlacesEditBookmarkKeywordTransaction.prototype = {
   __proto__: BaseTransaction.prototype,
 
   doTransaction: function EBKTXN_doTransaction()
   {
+    // Store the current values.
     this.item.keyword = PlacesUtils.bookmarks.getKeywordForBookmark(this.item.id);
+    if (this.item.keyword)
+      this.item.postData = PlacesUtils.getPostDataForBookmark(this.item.id);
+
+    // Update the keyword.
     PlacesUtils.bookmarks.setKeywordForBookmark(this.item.id, this.new.keyword);
+    if (this.new.keyword && this.new.postData)
+      PlacesUtils.setPostDataForBookmark(this.item.id, this.new.postData);
   },
 
   undoTransaction: function EBKTXN_undoTransaction()
   {
     PlacesUtils.bookmarks.setKeywordForBookmark(this.item.id, this.item.keyword);
+    if (this.item.postData)
+      PlacesUtils.setPostDataForBookmark(this.item.id, this.item.postData);
   }
 };
 
 
 /**
  * Transaction for editing the post data associated with a bookmark.
  *
  * @param aItemId
--- a/toolkit/components/places/tests/unit/test_placesTxn.js
+++ b/toolkit/components/places/tests/unit/test_placesTxn.js
@@ -535,27 +535,29 @@ add_task(function* test_edit_description
 });
 
 add_task(function* test_edit_keyword() {
   const KEYWORD = "keyword-test_edit_keyword";
 
   let testURI = NetUtil.newURI("http://test_edit_keyword.com");
   let testBkmId = bmsvc.insertBookmark(root, testURI, bmsvc.DEFAULT_INDEX, "Test edit keyword");
 
-  let txn = new PlacesEditBookmarkKeywordTransaction(testBkmId, KEYWORD);
+  let txn = new PlacesEditBookmarkKeywordTransaction(testBkmId, KEYWORD, "postData");
 
   txn.doTransaction();
   do_check_eq(observer._itemChangedId, testBkmId);
   do_check_eq(observer._itemChangedProperty, "keyword");
   do_check_eq(observer._itemChangedValue, KEYWORD);
+  do_check_eq(PlacesUtils.getPostDataForBookmark(testBkmId), "postData");
 
   txn.undoTransaction();
   do_check_eq(observer._itemChangedId, testBkmId);
   do_check_eq(observer._itemChangedProperty, "keyword");
   do_check_eq(observer._itemChangedValue, "");
+  do_check_eq(PlacesUtils.getPostDataForBookmark(testBkmId), null);
 });
 
 add_task(function* test_LoadInSidebar_transaction() {
   let testURI = NetUtil.newURI("http://test_LoadInSidebar_transaction.com");
   let testBkmId = bmsvc.insertBookmark(root, testURI, bmsvc.DEFAULT_INDEX, "Test LoadInSidebar transaction");
 
   const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
   let anno = { name: LOAD_IN_SIDEBAR_ANNO,
@@ -695,44 +697,16 @@ add_task(function* test_sort_folder_by_n
   do_check_eq(0, bmsvc.getItemIndex(b3));
 
   txn.undoTransaction();
   do_check_eq(0, bmsvc.getItemIndex(b1));
   do_check_eq(1, bmsvc.getItemIndex(b2));
   do_check_eq(2, bmsvc.getItemIndex(b3));
 });
 
-add_task(function* test_edit_postData() {
-  let postData = "post-test_edit_postData";
-  let testURI = NetUtil.newURI("http://test_edit_postData.com");
-
-  let testBkm = yield PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.menuGuid,
-    url: "http://test_edit_postData.com",
-    title: "Test edit Post Data"
-  });
-
-  yield PlacesUtils.keywords.insert({
-    keyword: "kw",
-    url: "http://test_edit_postData.com"
-  });
-
-  let testBkmId = yield PlacesUtils.promiseItemId(testBkm.guid);
-  let txn = new PlacesEditBookmarkPostDataTransaction(testBkmId, postData);
-
-  txn.doTransaction();
-  yield promiseKeyword("kw", testURI.spec, postData);
-
-  txn.undoTransaction();
-  entry = yield PlacesUtils.keywords.fetch("kw");
-  Assert.equal(entry.url.href, testURI.spec);
-  // We don't allow anymore to set a null post data.
-  //Assert.equal(null, post_data);
-});
-
 add_task(function* test_tagURI_untagURI() {
   const TAG_1 = "tag-test_tagURI_untagURI-bar";
   const TAG_2 = "tag-test_tagURI_untagURI-foo";
   let tagURI = NetUtil.newURI("http://test_tagURI_untagURI.com");
 
   // Test tagURI
   let tagTxn = new PlacesTagURITransaction(tagURI, [TAG_1, TAG_2]);