Bug 1345417 - Sync bookmarks postData. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Wed, 08 Mar 2017 14:48:27 -0500
changeset 495420 4466e47a4ad73ac9fbbb08558641ed0cc60b200b
parent 495323 800ba54a4bd52628833c4db005ddd182586666c4
child 548375 865b644c73830d2890feec504117173d495620d9
push id48330
push userbmo:eoger@fastmail.com
push dateWed, 08 Mar 2017 20:33:17 +0000
reviewersmarkh
bugs1345417
milestone55.0a1
Bug 1345417 - Sync bookmarks postData. r?markh MozReview-Commit-ID: 9if7ezpFvKu
services/sync/modules/engines/bookmarks.js
toolkit/components/places/PlacesSyncUtils.jsm
toolkit/components/places/PlacesUtils.jsm
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -149,34 +149,36 @@ Bookmark.prototype = {
   toSyncBookmark() {
     let info = PlacesItem.prototype.toSyncBookmark.call(this);
     info.title = this.title;
     info.url = this.bmkUri;
     info.description = this.description;
     info.loadInSidebar = this.loadInSidebar;
     info.tags = this.tags;
     info.keyword = this.keyword;
+    info.postData = this.postData;
     return info;
   },
 
   fromSyncBookmark(item) {
     PlacesItem.prototype.fromSyncBookmark.call(this, item);
     this.title = item.title;
     this.bmkUri = item.url.href;
     this.description = item.description;
     this.loadInSidebar = item.loadInSidebar;
     this.tags = item.tags;
     this.keyword = item.keyword;
+    this.postData = item.postData;
   },
 };
 
 Utils.deferGetSet(Bookmark,
                   "cleartext",
                   ["title", "bmkUri", "description",
-                   "loadInSidebar", "tags", "keyword"]);
+                   "loadInSidebar", "tags", "keyword", "postData"]);
 
 this.BookmarkQuery = function BookmarkQuery(collection, id) {
   Bookmark.call(this, collection, id, "query");
 }
 BookmarkQuery.prototype = {
   __proto__: Bookmark.prototype,
   _logName: "Sync.Record.BookmarkQuery",
 
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -567,16 +567,17 @@ const BookmarkSyncUtils = PlacesSyncUtil
    * The following properties are supported:
    *  - kind: Optional.
    *  - guid: Required.
    *  - parentGuid: Optional; reparents the bookmark if specified.
    *  - title: Optional.
    *  - url: Optional.
    *  - tags: Optional; replaces all existing tags.
    *  - keyword: Optional.
+   *  - postData: Optional.
    *  - description: Optional.
    *  - loadInSidebar: Optional.
    *  - query: Optional.
    *
    * @param info
    *        object representing a bookmark-item, as defined above.
    *
    * @return {Promise} resolved when the update is complete.
@@ -599,16 +600,17 @@ const BookmarkSyncUtils = PlacesSyncUtil
    * The following properties are supported:
    *  - kind: Required.
    *  - guid: Required.
    *  - parentGuid: Required.
    *  - url: Required for bookmarks.
    *  - query: A smart bookmark query string, optional.
    *  - tags: An optional array of tag strings.
    *  - keyword: An optional keyword string.
+   *  - postData: Optional.
    *  - description: An optional description string.
    *  - loadInSidebar: An optional boolean; defaults to false.
    *
    * Sync doesn't set the index, since it appends and reorders children
    * after applying all incoming items.
    *
    * @param info
    *        object representing a synced bookmark.
@@ -632,16 +634,17 @@ const BookmarkSyncUtils = PlacesSyncUtil
    *  - parentSyncId (all): The sync ID of the item's parent.
    *  - parentTitle (all): The title of the item's parent, used for de-duping.
    *    Omitted for the Places root and parents with empty titles.
    *  - title ("bookmark", "folder", "livemark", "query"): The item's title.
    *    Omitted if empty.
    *  - url ("bookmark", "query"): The item's URL.
    *  - tags ("bookmark", "query"): An array containing the item's tags.
    *  - keyword ("bookmark"): The bookmark's keyword, if one exists.
+   *  - postData ("bookmark"): The bookmark's postData, if it exists.
    *  - description ("bookmark", "folder", "livemark"): The item's description.
    *    Omitted if one isn't set.
    *  - loadInSidebar ("bookmark", "query"): Whether to load the bookmark in
    *    the sidebar. Always `false` for queries.
    *  - feed ("livemark"): A `URL` object pointing to the livemark's feed URL.
    *  - site ("livemark"): A `URL` object pointing to the livemark's site URL,
    *    or `null` if one isn't set.
    *  - childSyncIds ("folder"): An array containing the sync IDs of the item's
@@ -955,20 +958,19 @@ var insertSyncLivemark = Task.async(func
   }
 
   let livemarkItem = yield PlacesUtils.livemarks.addLivemark(livemarkInfo);
 
   return insertBookmarkMetadata(livemarkItem, insertInfo);
 });
 
 // Keywords are a 1 to 1 mapping between strings and pairs of (URL, postData).
-// (the postData is not synced, so we ignore it). Sync associates keywords with
-// bookmarks, which is not really accurate. -- We might already have a keyword
-// with that name, or we might already have another bookmark with that URL with
-// a different keyword, etc.
+// Sync associates keywords with bookmarks, which is not really accurate.
+// i.e., we might already have a keyword with that name, or we might already
+// have another bookmark with that URL with a different keyword, etc.
 //
 // If we don't handle those cases by removing the conflicting keywords first,
 // the insertion  will fail, and the keywords will either be wrong, or missing.
 // This function handles those cases.
 function removeConflictingKeywords(bookmarkURL, newKeyword) {
   return PlacesUtils.withConnectionWrapper(
     "BookmarkSyncUtils: removeConflictingKeywords", Task.async(function* (db) {
       let entryForURL = yield PlacesUtils.keywords.fetch({
@@ -1020,22 +1022,27 @@ var insertBookmarkMetadata = Task.async(
     newItem.tags = yield tagItem(bookmarkItem, insertInfo.tags);
   } catch (ex) {
     BookmarkSyncLog.warn(`insertBookmarkMetadata: Error tagging item ${
       insertInfo.syncId}`, ex);
   }
 
   if (insertInfo.keyword) {
     yield removeConflictingKeywords(bookmarkItem.url, insertInfo.keyword);
-    yield PlacesUtils.keywords.insert({
+    let keywordProps = {
       keyword: insertInfo.keyword,
       url: bookmarkItem.url.href,
       source: SOURCE_SYNC,
-    });
+    };
+    if (insertInfo.postData) {
+      keywordProps.postData = insertInfo.postData
+    }
+    yield PlacesUtils.keywords.insert(keywordProps);
     newItem.keyword = insertInfo.keyword;
+    newItem.postData = insertInfo.postData;
   }
 
   if (insertInfo.description) {
     PlacesUtils.annotations.setItemAnnotation(itemId,
       BookmarkSyncUtils.DESCRIPTION_ANNO, insertInfo.description, 0,
       PlacesUtils.annotations.EXPIRE_NEVER,
       SOURCE_SYNC);
     newItem.description = insertInfo.description;
@@ -1235,23 +1242,28 @@ var updateBookmarkMetadata = Task.async(
     BookmarkSyncLog.warn(`updateBookmarkMetadata: Error tagging item ${
       updateInfo.syncId}`, ex);
   }
 
   if (updateInfo.hasOwnProperty("keyword")) {
     // Unconditionally remove the old keyword.
     yield removeConflictingKeywords(oldBookmarkItem.url, updateInfo.keyword);
     if (updateInfo.keyword) {
-      yield PlacesUtils.keywords.insert({
+      let keywordProps = {
         keyword: updateInfo.keyword,
         url: newItem.url.href,
         source: SOURCE_SYNC,
-      });
+      };
+      if (updateInfo.postData) {
+        keywordProps.postData = updateInfo.postData;
+      }
+      yield PlacesUtils.keywords.insert(keywordProps);
     }
     newItem.keyword = updateInfo.keyword;
+    newItem.postData = updateInfo.postData;
   }
 
   if (updateInfo.hasOwnProperty("description")) {
     if (updateInfo.description) {
       PlacesUtils.annotations.setItemAnnotation(itemId,
         BookmarkSyncUtils.DESCRIPTION_ANNO, updateInfo.description, 0,
         PlacesUtils.annotations.EXPIRE_NEVER,
         SOURCE_SYNC);
@@ -1305,16 +1317,19 @@ function validateNewBookmark(info) {
     , query: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.QUERY }
     , folder: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.QUERY }
     , tags: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
                             , BookmarkSyncUtils.KINDS.MICROSUMMARY
                             , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , keyword: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
                                , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
+    , postData: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
+                                , BookmarkSyncUtils.KINDS.MICROSUMMARY
+                                , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , description: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
                                    , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                    , BookmarkSyncUtils.KINDS.QUERY
                                    , BookmarkSyncUtils.KINDS.FOLDER
                                    , BookmarkSyncUtils.KINDS.LIVEMARK ].includes(b.kind) }
     , loadInSidebar: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
                                      , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                      , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
@@ -1520,16 +1535,17 @@ var fetchBookmarkItem = Task.async(funct
   item.tags = PlacesUtils.tagging.getTagsForURI(
     PlacesUtils.toURI(bookmarkItem.url), {});
 
   let keywordEntry = yield PlacesUtils.keywords.fetch({
     url: bookmarkItem.url,
   });
   if (keywordEntry) {
     item.keyword = keywordEntry.keyword;
+    item.postData = keywordEntry.postData;
   }
 
   let description = yield getAnno(bookmarkItem.guid,
                                   BookmarkSyncUtils.DESCRIPTION_ANNO);
   if (description) {
     item.description = description;
   }
 
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -275,16 +275,17 @@ const SYNC_BOOKMARK_VALIDATORS = Object.
       if (typeof tag != "string" || !tag ||
           tag.length > Ci.nsITaggingService.MAX_TAG_LENGTH) {
         throw new Error(`Invalid tag: ${tag}`);
       }
     }
     return v;
   },
   keyword: simpleValidateFunc(v => v === null || typeof v == "string"),
+  postData: simpleValidateFunc(v => v === null || typeof v == "string"),
   description: simpleValidateFunc(v => v === null || typeof v == "string"),
   loadInSidebar: simpleValidateFunc(v => v === true || v === false),
   feed: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
   site: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
   title: BOOKMARK_VALIDATORS.title,
   url: BOOKMARK_VALIDATORS.url,
 });