Bug 1305563 - Correctly forward change sources when updating keywords. r?mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 10 Oct 2017 10:37:00 -0700
changeset 679408 9c6c3ab5c352aefdd873f2263ab4b2910cc50fc6
parent 679407 ce53235d12b265018dea7fd083ebcc4da603ad04
child 679409 c1832eb5c3488d3a10f7616fd9e90f40e4c3b650
push id84218
push userbmo:kit@mozilla.com
push dateThu, 12 Oct 2017 16:56:40 +0000
reviewersmak
bugs1305563
milestone58.0a1
Bug 1305563 - Correctly forward change sources when updating keywords. r?mak This patch fixes the keywords cache observer to forward the source for URL changes. Previously, Sync used the public `PlacesUtils.keywords` API, so this wasn't an issue. However, the new Sync bookmark buffer writes to `moz_keywords` directly, then fires `onItemChanged` observer notifications to update the cache. MozReview-Commit-ID: BMXvgTql9qb
toolkit/components/places/PlacesUtils.jsm
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2306,75 +2306,83 @@ XPCOMUtils.defineLazyGetter(this, "gKeyw
       let observer = {
         QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver),
         onBeginUpdateBatch() {},
         onEndUpdateBatch() {},
         onItemAdded() {},
         onItemVisited() {},
         onItemMoved() {},
 
-        onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid) {
+        onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid,
+                      source) {
           if (itemType != PlacesUtils.bookmarks.TYPE_BOOKMARK)
             return;
 
           let keywords = keywordsForHref(uri.spec);
           // This uri has no keywords associated, so there's nothing to do.
           if (keywords.length == 0)
             return;
 
           (async function() {
             // If the uri is not bookmarked anymore, we can remove this keyword.
             let bookmark = await PlacesUtils.bookmarks.fetch({ url: uri });
             if (!bookmark) {
               for (let keyword of keywords) {
-                await PlacesUtils.keywords.remove(keyword);
+                await PlacesUtils.keywords.remove({ keyword, source });
               }
             }
           })().catch(Cu.reportError);
         },
 
         onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid,
-                      parentGuid, oldVal) {
+                      parentGuid, oldVal, source) {
           if (gIgnoreKeywordNotifications) {
             return;
           }
 
           if (prop == "keyword") {
             this._onKeywordChanged(guid, val, oldVal);
           } else if (prop == "uri") {
-            this._onUrlChanged(guid, val, oldVal).catch(Cu.reportError);
+            this._onUrlChanged(guid, val, oldVal, source).catch(Cu.reportError);
           }
         },
 
         _onKeywordChanged(guid, keyword, href) {
           if (keyword.length == 0) {
             // We are removing a keyword.
             let keywords = keywordsForHref(href)
             for (let kw of keywords) {
               cache.delete(kw);
             }
           } else {
             // We are adding a new keyword.
             cache.set(keyword, { keyword, url: new URL(href) });
           }
         },
 
-        async _onUrlChanged(guid, url, oldUrl) {
+        async _onUrlChanged(guid, url, oldUrl, source) {
           // Check if the old url is associated with keywords.
           let entries = [];
           await PlacesUtils.keywords.fetch({ url: oldUrl }, e => entries.push(e));
           if (entries.length == 0) {
             return;
           }
 
           // Move the keywords to the new url.
           for (let entry of entries) {
-            await PlacesUtils.keywords.remove(entry.keyword);
-            entry.url = new URL(url);
-            await PlacesUtils.keywords.insert(entry);
+            await PlacesUtils.keywords.remove({
+              keyword: entry.keyword,
+              source,
+            });
+            await PlacesUtils.keywords.insert({
+              keyword: entry.keyword,
+              url,
+              postData: entry.postData,
+              source,
+            });
           }
         },
       };
 
       PlacesUtils.bookmarks.addObserver(observer);
       PlacesUtils.registerShutdownFunction(() => {
         PlacesUtils.bookmarks.removeObserver(observer);
       });