Bug 1343256 - Bookmark keywords disappear from one bookmark when adding a keyword to another bookmark. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Sat, 29 Apr 2017 15:15:06 +0200
changeset 573251 e2b4c5e2bd9baa8a30717c010bc63fb1ef2ae12c
parent 569138 0b77ed3f26c5335503bc16e85b8c067382e7bb1e
child 627231 713037a6ac8471f6f3570846f9b2cdd47bff887a
push id57307
push usermak77@bonardo.net
push dateFri, 05 May 2017 10:09:37 +0000
reviewersstandard8
bugs1343256
milestone55.0a1
Bug 1343256 - Bookmark keywords disappear from one bookmark when adding a keyword to another bookmark. r=standard8 MozReview-Commit-ID: Av8pDQi6Yyp
browser/components/places/content/editBookmarkOverlay.js
browser/components/places/tests/chrome/chrome.ini
browser/components/places/tests/chrome/test_editBookmarkOverlay_keywords.xul
toolkit/components/places/Bookmarks.jsm
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -133,35 +133,42 @@ var gEditItemOverlay = {
                         PlacesUIUtils.getItemDescription(this._paneInfo.itemId));
   },
 
   _initKeywordField: Task.async(function* (newKeyword = "") {
     if (!this._paneInfo.isBookmark) {
       throw new Error("_initKeywordField called unexpectedly");
     }
 
+    // Reset the field status synchronously now, eventually we'll reinit it
+    // later if we find an existing keyword. This way we can ensure to be in a
+    // consistent status when reusing the panel across different bookmarks.
+    this._keyword = newKeyword;
+    this._initTextField(this._keywordField, newKeyword);
+
     if (!newKeyword) {
       let entries = [];
       yield PlacesUtils.keywords.fetch({ url: this._paneInfo.uri.spec },
                                        e => entries.push(e));
       if (entries.length > 0) {
         // We show an existing keyword if either POST data was not provided, or
         // if the POST data is the same.
         let existingKeyword = entries[0].keyword;
         let postData = this._paneInfo.postData;
         if (postData) {
           let sameEntry = entries.find(e => e.postData === postData);
           existingKeyword = sameEntry ? sameEntry.keyword : "";
         }
         if (existingKeyword) {
-          this._keyword = newKeyword = existingKeyword;
+          this._keyword = existingKeyword;
+          // Update the text field to the existing keyword.
+          this._initTextField(this._keywordField, this._keyword);
         }
       }
     }
-    this._initTextField(this._keywordField, newKeyword);
   }),
 
   _initLoadInSidebar: Task.async(function* () {
     if (!this._paneInfo.isBookmark)
       throw new Error("_initLoadInSidebar called unexpectedly");
 
     this._loadInSidebarCheckbox.checked =
       PlacesUtils.annotations.itemHasAnnotation(
--- a/browser/components/places/tests/chrome/chrome.ini
+++ b/browser/components/places/tests/chrome/chrome.ini
@@ -4,11 +4,12 @@ support-files = head.js
 [test_0_bug510634.xul]
 [test_0_multiple_left_pane.xul]
 [test_bug1163447_selectItems_through_shortcut.xul]
 [test_bug427633_no_newfolder_if_noip.xul]
 [test_bug485100-change-case-loses-tag.xul]
 [test_bug549192.xul]
 [test_bug549491.xul]
 [test_bug631374_tags_selector_scroll.xul]
+[test_editBookmarkOverlay_keywords.xul]
 [test_editBookmarkOverlay_tags_liveUpdate.xul]
 [test_selectItems_on_nested_tree.xul]
 [test_treeview_date.xul]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/chrome/test_editBookmarkOverlay_keywords.xul
@@ -0,0 +1,99 @@
+<?xml version="1.0"?>
+
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+
+<?xml-stylesheet href="chrome://browser/skin/places/editBookmarkOverlay.css"?>
+<?xml-stylesheet href="chrome://browser/content/places/places.css"?>
+<?xml-stylesheet href="chrome://browser/skin/places/places.css"?>
+
+<?xul-overlay href="chrome://browser/content/places/placesOverlay.xul"?>
+<?xul-overlay href="chrome://browser/content/places/editBookmarkOverlay.xul"?>
+
+<!DOCTYPE window [
+  <!ENTITY % editBookmarkOverlayDTD SYSTEM "chrome://browser/locale/places/editBookmarkOverlay.dtd">
+  %editBookmarkOverlayDTD;
+]>
+
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        title="Bug 1343256 - Bookmark keywords disappear from one bookmark when adding a keyword to another bookmark"
+        onload="runTest();">
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
+  <script type="application/javascript"
+          src="chrome://browser/content/places/editBookmarkOverlay.js"/>
+
+  <body xmlns="http://www.w3.org/1999/xhtml" />
+
+  <vbox id="editBookmarkPanelContent"/>
+
+  <script type="application/javascript">
+  <![CDATA[
+    function runTest() {
+      SimpleTest.waitForExplicitFinish();
+      Task.spawn(test.bind(this))
+          .catch(ex => ok(false, ex))
+          .then(() => PlacesUtils.bookmarks.eraseEverything())
+          .then(SimpleTest.finish);
+    }
+
+    function promiseOnItemChanged() {
+      return new Promise(resolve => {
+        PlacesUtils.bookmarks.addObserver({
+          onBeginUpdateBatch() {},
+          onEndUpdateBatch() {},
+          onItemAdded() {},
+          onItemRemoved() {},
+          onItemVisited() {},
+          onItemMoved() {},
+          onItemChanged(id, property, isAnno, value) {
+            PlacesUtils.bookmarks.removeObserver(this);
+            resolve({ property, value });
+          },
+          QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarkObserver])
+        });
+      });
+    }
+
+    function* test() {
+      ok(gEditItemOverlay, "Sanity check: gEditItemOverlay is in context");
+      let keywordField = document.getElementById("editBMPanel_keywordField");
+
+      for (let i = 0; i < 2; ++i) {
+        let bm = yield PlacesUtils.bookmarks.insert({
+          url: `http://www.test${i}.me/`,
+          parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+        });
+        info(`Init panel on bookmark #${i+1}`);
+        let node = yield PlacesUIUtils.promiseNodeLikeFromFetchInfo(bm);
+        gEditItemOverlay.initPanel({ node });
+        is(document.getElementById("editBMPanel_keywordField").value, "",
+          "The keyword field should be empty");
+        info("Add a keyword to the bookmark");
+        let promise = promiseOnItemChanged();
+        keywordField.focus();
+        keywordField.value = "kw";
+        synthesizeKey(i.toString(), {});
+        synthesizeKey("VK_RETURN", {});
+        keywordField.blur();
+        let {property, value} = yield promise;
+        is(property, "keyword", "The keyword should have been changed");
+        is(value, `kw${i}`, "The new keyword value is correct");
+      }
+
+      for (let i = 0; i < 2; ++i) {
+        let entry = yield PlacesUtils.keywords.fetch({ url: `http://www.test${i}.me/` });
+        is(entry.keyword, `kw${i}`, `The keyword for http://www.test${i}.me/ is correct`);
+      }
+    };
+  ]]>
+  </script>
+
+</window>
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -670,32 +670,32 @@ var Bookmarks = Object.freeze({
    *        Additional options. Currently supports the following properties:
    *         - source: The change source, forwarded to all bookmark observers.
    *           Defaults to nsINavBookmarksService::SOURCE_DEFAULT.
    *
    * @return {Promise} resolved when the removal is complete.
    * @resolves once the removal is complete.
    */
   eraseEverything(options = {}) {
+    const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid,
+                          this.mobileGuid];
     return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: eraseEverything",
       db => db.executeTransaction(function* () {
-        const folderGuids = [this.toolbarGuid, this.menuGuid, this.unfiledGuid,
-                             this.mobileGuid];
         yield removeFoldersContents(db, folderGuids, options);
         const time = PlacesUtils.toPRTime(new Date());
         const syncChangeDelta =
           PlacesSyncUtils.bookmarks.determineSyncChangeDelta(options.source);
         for (let folderGuid of folderGuids) {
           yield db.executeCached(
             `UPDATE moz_bookmarks SET lastModified = :time,
                                       syncChangeCounter = syncChangeCounter + :syncChangeDelta
              WHERE id IN (SELECT id FROM moz_bookmarks WHERE guid = :folderGuid )
             `, { folderGuid, time, syncChangeDelta });
         }
-      }.bind(this))
+      })
     );
   },
 
   /**
    * Returns a list of recently bookmarked items.
    * Only includes actual bookmarks. Excludes folders, separators and queries.
    *
    * @param {integer} numberOfItems