Bug 1167915 - "Add a Keyword for this Search" does not work anymore on POST forms. r=ttaubert, a=lizzard
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 08 Jun 2015 13:48:09 -0400
changeset 266207 d810a18a0e0f
parent 266206 0c1d5e2461d4
child 266208 7cee52e60929
push id4787
push userryanvm@gmail.com
push date2015-06-08 17:52 +0000
treeherdermozilla-beta@48c9f45a00f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, lizzard
bugs1167915
milestone39.0
Bug 1167915 - "Add a Keyword for this Search" does not work anymore on POST forms. r=ttaubert, a=lizzard
browser/base/content/browser.js
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/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6569,17 +6569,17 @@ function AddKeywordForSearchField() {
 
   PlacesUIUtils.showBookmarkDialog({ action: "add"
                                    , type: "bookmark"
                                    , uri: makeURI(bookmarkData.spec)
                                    , title: bookmarkData.title
                                    , description: bookmarkData.description
                                    , keyword: ""
                                    , postData: bookmarkData.postData
-                                   , charSet: bookmarkData.charset
+                                   , charSet: bookmarkData.charSet
                                    , hiddenRows: [ "location"
                                                  , "description"
                                                  , "tags"
                                                  , "loadInSidebar" ]
                                    }, window);
 }
 
 function convertFromUnicode(charset, str)
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -1305,19 +1305,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
@@ -184,21 +184,21 @@ var BookmarkPropertiesPanel = {
           }
 
           if ("loadBookmarkInSidebar" in dialogInfo)
             this._loadInSidebar = dialogInfo.loadBookmarkInSidebar;
 
           if ("keyword" in dialogInfo) {
             this._keyword = dialogInfo.keyword;
             this._isAddKeywordDialog = true;
-            if ("postData" in dialogInfo)
-              this._postData = dialogInfo.postData;
             if ("charSet" in dialogInfo)
               this._charSet = dialogInfo.charSet;
           }
+          if ("postData" in dialogInfo)
+            this._postData = dialogInfo.postData;
           break;
 
         case "folder":
           this._itemType = BOOKMARK_FOLDER;
           if (!this._title) {
             if ("URIList" in dialogInfo) {
               this._title = this._strings.getString("bookmarkAllTabsDefault");
               this._URIs = dialogInfo.URIList;
@@ -418,17 +418,18 @@ var BookmarkPropertiesPanel = {
                                { hiddenRows: this._hiddenRows,
                                  forceReadOnly: this._readOnly });
   },
 
   _fillAddProperties: function BPP__fillAddProperties() {
     this._createNewItem();
     // Edit the new item
     gEditItemOverlay.initPanel(this._itemId,
-                               { 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.
     var locationField = this._element("locationField");
     if (locationField.value == "about:blank")
       locationField.value = "";
   },
 
@@ -551,32 +552,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
@@ -45,16 +45,17 @@ var gEditItemOverlay = {
     if (aInfo && aInfo.hiddenRows)
       this._hiddenRows = aInfo.hiddenRows;
     else
       this._hiddenRows.splice(0, this._hiddenRows.length);
     // force-read-only
     this._readOnly = aInfo && aInfo.forceReadOnly;
     this._titleOverride = aInfo && aInfo.titleOverride ? aInfo.titleOverride
                                                        : "";
+    this._postData = aInfo && aInfo.postData;
   },
 
   _showHideRows: function EIO__showHideRows() {
     var isBookmark = this._itemId != -1 &&
                      this._itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK;
     var isQuery = false;
     if (this._uri)
       isQuery = this._uri.schemeIs("place");
@@ -585,17 +586,17 @@ var gEditItemOverlay = {
       PlacesUtils.transactionManager.doTransaction(txn);
       this._uri = uri;
     }
   },
 
   onKeywordFieldBlur: function EIO_onKeywordFieldBlur() {
     var keyword = this._element("keywordField").value;
     if (keyword != PlacesUtils.bookmarks.getKeywordForBookmark(this._itemId)) {
-      var txn = new PlacesEditBookmarkKeywordTransaction(this._itemId, keyword);
+      var txn = new PlacesEditBookmarkKeywordTransaction(this._itemId, keyword, this._postData);
       PlacesUtils.transactionManager.doTransaction(txn);
     }
   },
 
   onLoadInSidebarCheckboxCommand:
   function EIO_onLoadInSidebarCheckboxCommand() {
     let annoObj = { name : PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO };
     if (this._element("loadInSidebarCheckbox").checked)
--- 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,55 @@
+"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(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,138 @@ 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");
+  // The dialog might be modal and could block our events loop, so executeSoon.
+  executeSoon(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
@@ -2629,46 +2629,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;
@@ -2945,16 +2952,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);
@@ -2996,16 +3005,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
@@ -3244,40 +3256,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
@@ -537,27 +537,29 @@ add_test(function test_edit_description_
 });
 
 add_test(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);
 
   run_next_test();
 });
 
 add_test(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");
 
@@ -711,57 +713,16 @@ add_test(function test_sort_folder_by_na
   txn.undoTransaction();
   do_check_eq(0, bmsvc.getItemIndex(b1));
   do_check_eq(1, bmsvc.getItemIndex(b2));
   do_check_eq(2, bmsvc.getItemIndex(b3));
 
   run_next_test();
 });
 
-add_test(function test_edit_postData() {
-  function* promiseKeyword(keyword, href, postData) {
-    while (true) {
-      let entry = yield PlacesUtils.keywords.fetch(keyword);
-      if (entry && entry.url.href == href && entry.postData == postData) {
-        break;
-      }
-
-      yield new Promise(resolve => do_timeout(100, resolve));
-    }
-  }
-
-  Task.spawn(function* () {
-    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);
-  }).then(run_next_test);
-});
-
 add_test(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]);