Bug 1103978 - aNewValue=undefined in onItemChange for removed keyword. r=mak
authorAsaf Romano <mano@mozilla.com>
Wed, 26 Nov 2014 06:16:44 +0200
changeset 217672 0c112347e32268528bba051b29b45a6a4ad46c55
parent 217671 4f555b40f01e328c75dffcc62b6b16886105c570
child 217673 b8d490a607e7bebddbca7dd9dc473ebc86c247ec
push id27887
push userryanvm@gmail.com
push dateThu, 27 Nov 2014 02:08:38 +0000
treeherdermozilla-central@c63e741bca2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1103978
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1103978 - aNewValue=undefined in onItemChange for removed keyword. r=mak
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
toolkit/components/places/tests/unit/test_async_transactions.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -357,18 +357,21 @@ let Bookmarks = Object.freeze({
                                              false, updatedItem.url.href,
                                              toPRTime(updatedItem.lastModified),
                                              updatedItem.type,
                                              updatedItem._parentId,
                                              updatedItem.guid,
                                              updatedItem.parentGuid ]);
       }
       if (updateInfo.hasOwnProperty("keyword")) {
+        // If the keyword is unset, updatedItem won't have it set.
+        let keyword = updatedItem.hasOwnProperty("keyword") ?
+                        updatedItem.keyword : "";
         notify(observers, "onItemChanged", [ updatedItem._id, "keyword",
-                                             false, updatedItem.keyword,
+                                             false, keyword,
                                              toPRTime(updatedItem.lastModified),
                                              updatedItem.type,
                                              updatedItem._parentId,
                                              updatedItem.guid,
                                              updatedItem.parentGuid ]);
       }
       // If the item was moved, notify onItemMoved.
       if (item.parentGuid != updatedItem.parentGuid ||
@@ -774,20 +777,24 @@ function* updateBookmark(info, item, new
   let tuples = new Map();
   if (info.hasOwnProperty("lastModified"))
     tuples.set("lastModified", { value: toPRTime(info.lastModified) });
   if (info.hasOwnProperty("title"))
     tuples.set("title", { value: info.title });
 
   yield db.executeTransaction(function* () {
     if (info.hasOwnProperty("keyword")) {
-      if (info.keyword.length > 0)
+      if (info.keyword.length > 0) {
         yield maybeCreateKeyword(db, info.keyword);
-      tuples.set("keyword", { value: info.keyword
-                            , fragment: "keyword_id = (SELECT id FROM moz_keywords WHERE keyword = :keyword)" });
+        tuples.set("keyword",
+                   { value: info.keyword
+                   , fragment: "keyword_id = (SELECT id FROM moz_keywords WHERE keyword = :keyword)" });
+      } else {
+        tuples.set("keyword_id", { value: null });
+      }
     }
 
     if (info.hasOwnProperty("url")) {
       // Ensure a page exists in moz_places for this URL.
       yield db.executeCached(
         `INSERT OR IGNORE INTO moz_places (url, rev_host, hidden, frecency, guid) 
          VALUES (:url, :rev_host, 0, :frecency, GENERATE_GUID())
         `, { url: info.url ? info.url.href : null,
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
@@ -168,26 +168,33 @@ add_task(function* update_bookmark_uri()
                  ]);
 });
 
 add_task(function* update_bookmark_keyword() {
   let bm = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://keyword.example.com/") });
   let observer = expectNotifications();
-  bm = yield PlacesUtils.bookmarks.update({ guid: bm.guid,
-                                            keyword: "kw" });
+  bm = yield PlacesUtils.bookmarks.update({ guid: bm.guid, keyword: "kw" });
   let itemId = yield PlacesUtils.promiseItemId(bm.guid);
   let parentId = yield PlacesUtils.promiseItemId(bm.parentGuid);
 
   observer.check([ { name: "onItemChanged",
                      arguments: [ itemId, "keyword", false, bm.keyword,
                                   bm.lastModified, bm.type, parentId, bm.guid,
                                   bm.parentGuid ] }
                  ]);
+
+  observer = expectNotifications();
+  bm = yield PlacesUtils.bookmarks.update({ guid: bm.guid, keyword: "" });
+  observer.check([ { name: "onItemChanged",
+                     arguments: [ itemId, "keyword", false, "",
+                                  bm.lastModified, bm.type, parentId, bm.guid,
+                                  bm.parentGuid ] }
+                 ]);
 });
 
 add_task(function* update_move_same_folder() {
   // Ensure there are at least two items in place (others test do so for us,
   // but we don't have to depend on that).
   let sep = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
                                                  parentGuid: PlacesUtils.bookmarks.unfiledGuid });
   let bm = yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -62,16 +62,17 @@ let observer = {
     this.itemsRemoved.set(aGuid, { parentGuid: aParentGuid
                                  , index:      aIndex
                                  , itemType:   aItemType });
   },
 
   onItemChanged:
   function (aItemId, aProperty, aIsAnnoProperty, aNewValue, aLastModified,
             aItemType, aParentId, aGuid, aParentGuid) {
+    dump("\n\n\n\nOnItemChange: " + aProperty + " " + aNewValue + "\n\n");
     if (this.tagRelatedGuids.has(aGuid))
       return;
 
     let changesForGuid = this.itemsChanged.get(aGuid);
     if (changesForGuid === undefined) {
       changesForGuid = new Map();
       this.itemsChanged.set(aGuid, changesForGuid);
     }