Bug 1103978 - aNewValue=undefined in onItemChange for removed keyword. r=mak

toolkit/components/places/Bookmarks.jsm

@@ -362,8 +362,11 @@ let Bookmarks = Object.freeze({
                                              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,
@@ -779,10 +782,14 @@ function* updateBookmark(info, item, newParent) {
 
   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")) {

toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js

@@ -173,8 +173,7 @@ add_task(function* update_bookmark_keyword() {
                                                 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);
 
@@ -183,6 +182,14 @@ add_task(function* update_bookmark_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() {

toolkit/components/places/tests/unit/test_async_transactions.js

@@ -67,6 +67,7 @@ let observer = {
   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;