Bug 1840207 - The bookmarked-moved event needs all the details to do a remove+insert operation. r=daisuke

Differential Revision: https://phabricator.services.mozilla.com/D182595

browser/components/places/tests/browser/browser_library_views_liveupdate.js

@@ -10,7 +10,6 @@ let gLibrary = null;
 
 add_setup(async function () {
   gLibrary = await promiseLibrary();
-
   await PlacesUtils.bookmarks.eraseEverything();
 
   registerCleanupFunction(async () => {
@@ -22,13 +21,16 @@ add_setup(async function () {
 async function testInFolder(folderGuid, prefix) {
   let addedBookmarks = [];
 
-  let item = await insertAndCheckItem({
+  let item = await insertAndCheckItem(
-    parentGuid: folderGuid,
+    {
-    title: `${prefix}1`,
+      parentGuid: folderGuid,
-    url: `http://${prefix}1.mozilla.org/`,
+      title: `${prefix}1`,
-  });
+      url: `http://${prefix}1.mozilla.org/`,
+    },
+    0
+  );
   item.title = `${prefix}1_edited`;
-  await updateAndCheckItem(item);
+  await updateAndCheckItem(item, 0);
   addedBookmarks.push(item);
 
   item = await insertAndCheckItem(
@@ -44,10 +46,13 @@ async function testInFolder(folderGuid, prefix) {
   await updateAndCheckItem(item, 0);
   addedBookmarks.push(item);
 
-  item = await insertAndCheckItem({
+  item = await insertAndCheckItem(
-    parentGuid: folderGuid,
+    {
-    type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+      parentGuid: folderGuid,
-  });
+      type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+    },
+    2
+  );
   addedBookmarks.push(item);
 
   item = await insertAndCheckItem(
@@ -68,22 +73,30 @@ async function testInFolder(folderGuid, prefix) {
 
   let folderGuid1 = item.guid;
 
-  item = await insertAndCheckItem({
+  item = await insertAndCheckItem(
-    parentGuid: folderGuid1,
+    {
-    title: `${prefix}f1`,
+      parentGuid: folderGuid1,
-    url: `http://${prefix}f1.mozilla.org/`,
+      title: `${prefix}f1`,
-  });
+      url: `http://${prefix}f1.mozilla.org/`,
+    },
+    0
+  );
   addedBookmarks.push(item);
 
-  item = await insertAndCheckItem({
+  item = await insertAndCheckItem(
-    parentGuid: folderGuid1,
+    {
-    title: `${prefix}f12`,
+      parentGuid: folderGuid,
-    url: `http://${prefix}f12.mozilla.org/`,
+      title: `${prefix}f12`,
-  });
+      url: `http://${prefix}f12.mozilla.org/`,
+    },
+    4
+  );
   addedBookmarks.push(item);
 
+  // Move to a different folder and index.
+  item.parentGuid = folderGuid1;
   item.index = 0;
-  await updateAndCheckItem(item);
+  await updateAndCheckItem(item, 0);
 
   return addedBookmarks;
 }
@@ -112,144 +125,93 @@ add_task(async function test() {
   }
 });
 
-async function insertAndCheckItem(itemData, expectedIndex) {
+function selectItem(item, parentTreeIndex) {
-  let item = await PlacesUtils.bookmarks.insert(itemData);
+  let useLeftPane =
+    item.type == PlacesUtils.bookmarks.TYPE_FOLDER || // is Folder
+    (item.type == PlacesUtils.bookmarks.TYPE_BOOKMARK &&
+      item.url.protocol == "place:"); // is Query
+  let tree = useLeftPane
+    ? gLibrary.PlacesOrganizer._places
+    : gLibrary.ContentTree.view;
+  tree.selectItems([item.guid]);
+  let treeIndex = tree.view.treeIndexForNode(tree.selectedNode);
+  let title = tree.view.getCellText(treeIndex, tree.columns.getColumnAt(0));
+  if (useLeftPane) {
+    // Make the treeIndex relative to the parent, otherwise getting the right
+    // index value is tricky due to separators and URIs being hidden in the left
+    // pane.
+    treeIndex -= parentTreeIndex + 1;
+  }
+  return {
+    node: tree.selectedNode,
+    title,
+    parentRelativeTreeIndex: treeIndex,
+  };
+}
 
-  let [node, index, title] = getNodeForTreeItem(
+function itemExists(item) {
-    item.guid,
+  let useLeftPane =
-    gLibrary.PlacesOrganizer._places
+    item.type == PlacesUtils.bookmarks.TYPE_FOLDER || // is Folder
+    (item.type == PlacesUtils.bookmarks.TYPE_BOOKMARK &&
+      item.url.protocol == "place:"); // is Query
+  let tree = useLeftPane
+    ? gLibrary.PlacesOrganizer._places
+    : gLibrary.ContentTree.view;
+  tree.selectItems([item.guid], true);
+  return tree.selectedNode?.bookmarkGuid == item.guid;
+}
+
+async function insertAndCheckItem(insertItem, expectedParentRelativeIndex) {
+  // Ensure the parent is selected before the change, this covers live updating
+  // better than selecting the parent later, that would just refresh all its
+  // children.
+  Assert.ok(insertItem.parentGuid, "Must have a parentGuid");
+  gLibrary.PlacesOrganizer._places.selectItems([insertItem.parentGuid], true);
+  let tree = gLibrary.PlacesOrganizer._places;
+  let parentTreeIndex = tree.view.treeIndexForNode(tree.selectedNode);
+
+  let item = await PlacesUtils.bookmarks.insert(insertItem);
+
+  let { node, title, parentRelativeTreeIndex } = selectItem(
+    item,
+    parentTreeIndex
+  );
+  Assert.equal(item.guid, node.bookmarkGuid, "Should find the updated node");
+  Assert.equal(title, item.title, "Should have the correct title");
+  Assert.equal(
+    parentRelativeTreeIndex,
+    expectedParentRelativeIndex,
+    "Should have the expected index"
   );
-  // Left pane should not be updated for normal bookmarks or separators.
-  switch (itemData.type || PlacesUtils.bookmarks.TYPE_BOOKMARK) {
-    case PlacesUtils.bookmarks.TYPE_BOOKMARK:
-      let uriString = itemData.url;
-      let isQuery = uriString.substr(0, 6) == "place:";
-      if (isQuery) {
-        Assert.ok(node, "Should have a new query in the left pane.");
-        break;
-      }
-    // Fallthrough if this isn't a query
-    case PlacesUtils.bookmarks.TYPE_SEPARATOR:
-      Assert.ok(
-        !node,
-        "Should not have added a bookmark or separator to the left pane."
-      );
-      break;
-    default:
-      Assert.ok(
-        node,
-        "Should have added a new node in the left pane for a folder."
-      );
-  }
-
-  if (node) {
-    Assert.equal(title, itemData.title, "Should have the correct title");
-    Assert.equal(index, expectedIndex, "Should have the expected index");
-  }
-
   return item;
 }
 
-async function updateAndCheckItem(newItemData, expectedIndex) {
+async function updateAndCheckItem(updateItem, expectedParentRelativeTreeIndex) {
-  await PlacesUtils.bookmarks.update(newItemData);
+  // Ensure the parent is selected before the change, this covers live updating
+  // better than selecting the parent later, that would just refresh all its
+  // children.
+  Assert.ok(updateItem.parentGuid, "Must have a parentGuid");
+  gLibrary.PlacesOrganizer._places.selectItems([updateItem.parentGuid], true);
+  let tree = gLibrary.PlacesOrganizer._places;
+  let parentTreeIndex = tree.view.treeIndexForNode(tree.selectedNode);
 
-  let [node, index, title] = getNodeForTreeItem(
+  let item = await PlacesUtils.bookmarks.update(updateItem);
-    newItemData.guid,
+
-    gLibrary.PlacesOrganizer._places
+  let { node, title, parentRelativeTreeIndex } = selectItem(
+    item,
+    parentTreeIndex
   );
-
+  Assert.equal(item.guid, node.bookmarkGuid, "Should find the updated node");
-  // Left pane should not be updated for normal bookmarks or separators.
+  Assert.equal(title, item.title, "Should have the correct title");
-  switch (newItemData.type || PlacesUtils.bookmarks.TYPE_BOOKMARK) {
+  Assert.equal(
-    case PlacesUtils.bookmarks.TYPE_BOOKMARK:
+    parentRelativeTreeIndex,
-      let isQuery = newItemData.url.protocol == "place:";
+    expectedParentRelativeTreeIndex,
-      if (isQuery) {
+    "Should have the expected index"
-        Assert.ok(node, "Should be able to find the updated node");
+  );
-        break;
+  return item;
-      }
-    // Fallthrough if this isn't a query
-    case PlacesUtils.bookmarks.TYPE_SEPARATOR:
-      Assert.ok(!node, "Should not be able to find the updated node");
-      break;
-    default:
-      Assert.ok(node, "Should be able to find the updated node");
-  }
-
-  if (node) {
-    Assert.equal(title, newItemData.title, "Should have the correct title");
-    Assert.equal(index, expectedIndex, "Should have the expected index");
-  }
 }
 
 async function removeAndCheckItem(itemData) {
   await PlacesUtils.bookmarks.remove(itemData);
-  let [node] = getNodeForTreeItem(
+  Assert.ok(!itemExists(itemData), "Should not find the updated node");
-    itemData.guid,
-    gLibrary.PlacesOrganizer._places
-  );
-  Assert.ok(!node, "Should not be able to find the removed node");
-}
-
-/**
- * Get places node, index and cell text for a guid in a tree view.
- *
- * @param {string} aItemGuid
- *        item guid of the item to search.
- * @param {object} aTree
- *        Tree to search in.
- * @returns {Array}
- *          [node, index, cellText] or [null, null, ""] if not found.
- */
-function getNodeForTreeItem(aItemGuid, aTree) {
-  function findNode(aContainerIndex) {
-    if (aTree.view.isContainerEmpty(aContainerIndex)) {
-      return [null, null, ""];
-    }
-
-    // The rowCount limit is just for sanity, but we will end looping when
-    // we have checked the last child of this container or we have found node.
-    for (let i = aContainerIndex + 1; i < aTree.view.rowCount; i++) {
-      let node = aTree.view.nodeForTreeIndex(i);
-
-      if (node.bookmarkGuid == aItemGuid) {
-        // Minus one because we want relative index inside the container.
-        let tree = gLibrary.PlacesOrganizer._places;
-        let cellText = tree.view.getCellText(i, tree.columns.getColumnAt(0));
-        return [node, i - aContainerIndex - 1, cellText];
-      }
-
-      if (PlacesUtils.nodeIsFolder(node)) {
-        // Open container.
-        aTree.view.toggleOpenState(i);
-        // Search inside it.
-        let foundNode = findNode(i);
-        // Close container.
-        aTree.view.toggleOpenState(i);
-        // Return node if found.
-        if (foundNode[0] != null) {
-          return foundNode;
-        }
-      }
-
-      // We have finished walking this container.
-      if (!aTree.view.hasNextSibling(aContainerIndex + 1, i)) {
-        break;
-      }
-    }
-    return [null, null, ""];
-  }
-
-  // Root node is hidden, so we need to manually walk the first level.
-  for (let i = 0; i < aTree.view.rowCount; i++) {
-    // Open container.
-    aTree.view.toggleOpenState(i);
-    // Search inside it.
-    let foundNode = findNode(i);
-    // Close container.
-    aTree.view.toggleOpenState(i);
-    // Return node if found.
-    if (foundNode[0] != null) {
-      return foundNode;
-    }
-  }
-  return [null, null, ""];
 }

dom/base/PlacesBookmarkMoved.h

@@ -29,6 +29,16 @@ class PlacesBookmarkMoved final : public PlacesBookmark {
     event->mOldIndex = aInitDict.mOldIndex;
     event->mSource = aInitDict.mSource;
     event->mIsTagging = aInitDict.mIsTagging;
+    event->mTitle = aInitDict.mTitle;
+    event->mTags = aInitDict.mTags;
+    event->mFrecency = aInitDict.mFrecency;
+    event->mHidden = aInitDict.mHidden;
+    event->mVisitCount = aInitDict.mVisitCount;
+    if (!aInitDict.mLastVisitDate.IsNull()) {
+      event->mLastVisitDate.SetValue(aInitDict.mLastVisitDate.Value());
+    } else {
+      event->mLastVisitDate.SetNull();
+    }
     return event.forget();
   }
 
@@ -46,10 +56,22 @@ class PlacesBookmarkMoved final : public PlacesBookmark {
   void GetOldParentGuid(nsCString& aOldParentGuid) {
     aOldParentGuid = mOldParentGuid;
   }
+  void GetTitle(nsString& aTitle) { aTitle = mTitle; }
+  void GetTags(nsString& aTags) { aTags = mTags; }
+  uint64_t Frecency() { return mFrecency; }
+  bool Hidden() { return mHidden; }
+  uint32_t VisitCount() { return mVisitCount; }
+  Nullable<uint64_t> GetLastVisitDate() { return mLastVisitDate; }
 
   int32_t mIndex;
   int32_t mOldIndex;
   nsCString mOldParentGuid;
+  nsString mTitle;
+  nsString mTags;
+  int64_t mFrecency;
+  bool mHidden;
+  uint32_t mVisitCount;
+  Nullable<uint64_t> mLastVisitDate;
 
  private:
   ~PlacesBookmarkMoved() = default;

dom/chrome-webidl/PlacesEvent.webidl

@@ -297,6 +297,12 @@ dictionary PlacesBookmarkMovedInit {
   required long oldIndex;
   required unsigned short source;
   required boolean isTagging;
+  required DOMString title;
+  required DOMString? tags;
+  required long long frecency;
+  required boolean hidden;
+  required unsigned long visitCount;
+  required unsigned long long? lastVisitDate;
 };
 
 [ChromeOnly, Exposed=Window]
@@ -320,6 +326,36 @@ interface PlacesBookmarkMoved : PlacesBookmark {
    * The item's old index in the folder.
    */
   readonly attribute long oldIndex;
+
+  /**
+   * The title of the added item.
+   */
+  readonly attribute DOMString title;
+
+  /**
+   * The tags of the added item.
+   */
+  readonly attribute DOMString tags;
+
+  /**
+   * The frecency of the page of this bookmark.
+   */
+  readonly attribute long long frecency;
+
+  /**
+   * Whether the visited page is marked as hidden.
+   */
+  readonly attribute boolean hidden;
+
+  /**
+   * Number of visits (including this one) for this URL.
+   */
+  readonly attribute unsigned long visitCount;
+
+  /**
+   * Date of the last visit, in milliseconds since epoch.
+   */
+  readonly attribute unsigned long long? lastVisitDate;
 };
 
 [ChromeOnly, Exposed=Window]

toolkit/components/places/Bookmarks.sys.mjs

@@ -935,6 +935,9 @@ export var Bookmarks = Object.freeze({
             item.parentGuid != updatedItem.parentGuid ||
             item.index != updatedItem.index
           ) {
+            let details = (await getBookmarkDetailMap([updatedItem.guid])).get(
+              updatedItem.guid
+            );
             notifications.push(
               new PlacesBookmarkMoved({
                 id: updatedItem._id,
@@ -949,6 +952,12 @@ export var Bookmarks = Object.freeze({
                 isTagging:
                   updatedItem.parentGuid === Bookmarks.tagsGuid ||
                   parent.parentGuid === Bookmarks.tagsGuid,
+                title: updatedItem.title,
+                tags: details.tags,
+                frecency: details.frecency,
+                hidden: details.hidden,
+                visitCount: details.visitCount,
+                lastVisitDate: details.lastVisitDate,
               })
             );
           }
@@ -1160,7 +1169,9 @@ export var Bookmarks = Object.freeze({
       );
 
       const notifications = [];
-
+      let detailsMap = await getBookmarkDetailMap(
+        updateInfos.map(({ updatedItem }) => updatedItem.guid)
+      );
       // Updates complete, time to notify everyone.
       for (let { updatedItem, existingItem, newParent } of updateInfos) {
         // If the item was moved, notify bookmark-moved.
@@ -1171,6 +1182,7 @@ export var Bookmarks = Object.freeze({
           existingItem.parentGuid != updatedItem.parentGuid ||
           existingItem.index != updatedItem.index
         ) {
+          let details = detailsMap.get(updatedItem.guid);
           notifications.push(
             new PlacesBookmarkMoved({
               id: updatedItem._id,
@@ -1185,6 +1197,12 @@ export var Bookmarks = Object.freeze({
               isTagging:
                 updatedItem.parentGuid === Bookmarks.tagsGuid ||
                 newParent.parentGuid === Bookmarks.tagsGuid,
+              title: updatedItem.title,
+              tags: details.tags,
+              frecency: details.frecency,
+              hidden: details.hidden,
+              visitCount: details.visitCount,
+              lastVisitDate: details.lastVisitDate,
             })
           );
         }
@@ -1758,7 +1776,11 @@ export var Bookmarks = Object.freeze({
       );
 
       const notifications = [];
+      let detailsMap = await getBookmarkDetailMap(
+        sortedChildren.map(c => c.guid)
+      );
       for (let child of sortedChildren) {
+        let details = detailsMap.get(child.guid);
         notifications.push(
           new PlacesBookmarkMoved({
             id: child._id,
@@ -1773,6 +1795,12 @@ export var Bookmarks = Object.freeze({
             isTagging:
               child.parentGuid === Bookmarks.tagsGuid ||
               parent.parentGuid === Bookmarks.tagsGuid,
+            title: child.title,
+            tags: details.tags,
+            frecency: details.frecency,
+            hidden: details.hidden,
+            visitCount: details.visitCount,
+            lastVisitDate: details.lastVisitDate,
           })
         );
       }

toolkit/components/places/SyncedBookmarksMirror.sys.mjs

@@ -2228,11 +2228,25 @@ class BookmarkObserverRecorder {
       `SELECT b.id, b.guid, b.type, p.guid AS newParentGuid, c.oldParentGuid,
               b.position AS newPosition, c.oldPosition,
               gp.guid AS grandParentGuid,
-              (SELECT h.url FROM moz_places h WHERE h.id = b.fk) AS url
+              h.url AS url, IFNULL(b.title, '') AS title,
+              IFNULL(h.frecency, 0) AS frecency, IFNULL(h.hidden, 0) AS hidden,
+              IFNULL(h.visit_count, 0) AS visit_count,
+              h.last_visit_date,
+              (
+                SELECT GROUP_CONCAT(t.title, ',')
+                FROM moz_bookmarks t
+                LEFT JOIN moz_bookmarks ref ON ref.fk = h.id
+                WHERE t.id = +ref.parent
+                  AND t.parent = (
+                    SELECT id FROM moz_bookmarks
+                    WHERE guid = '${lazy.PlacesUtils.bookmarks.tagsGuid}'
+                  )
+              ) AS tags
        FROM itemsMoved c
        JOIN moz_bookmarks b ON b.id = c.itemId
        JOIN moz_bookmarks p ON p.id = b.parent
        LEFT JOIN moz_bookmarks gp ON gp.id = p.parent
+       LEFT JOIN moz_places h ON h.id = b.fk
        ${this.orderBy("c.level", "b.parent", "b.position")}`,
       null,
       (row, cancel) => {
@@ -2240,6 +2254,7 @@ class BookmarkObserverRecorder {
           cancel();
           return;
         }
+        let lastVisitDate = row.getResultByName("last_visit_date");
         let info = {
           id: row.getResultByName("id"),
           guid: row.getResultByName("guid"),
@@ -2250,6 +2265,14 @@ class BookmarkObserverRecorder {
           oldPosition: row.getResultByName("oldPosition"),
           urlHref: row.getResultByName("url"),
           grandParentGuid: row.getResultByName("grandParentGuid"),
+          title: row.getResultByName("title"),
+          frecency: row.getResultByName("frecency"),
+          hidden: row.getResultByName("hidden"),
+          visitCount: row.getResultByName("visit_count"),
+          lastVisitDate: lastVisitDate
+            ? lazy.PlacesUtils.toDate(lastVisitDate).getTime()
+            : null,
+          tags: row.getResultByName("tags"),
         };
         this.noteItemMoved(info);
       }
@@ -2359,6 +2382,7 @@ class BookmarkObserverRecorder {
         id: info.id,
         itemType: info.type,
         url: info.urlHref,
+        title: info.title,
         guid: info.guid,
         parentGuid: info.newParentGuid,
         source: lazy.PlacesUtils.bookmarks.SOURCES.SYNC,
@@ -2368,6 +2392,11 @@ class BookmarkObserverRecorder {
         isTagging:
           info.newParentGuid === lazy.PlacesUtils.bookmarks.tagsGuid ||
           info.grandParentGuid === lazy.PlacesUtils.bookmarks.tagsGuid,
+        tags: info.tags,
+        frecency: info.frecency,
+        hidden: info.hidden,
+        visitCount: info.visitCount,
+        lastVisitDate: info.lastVisitDate,
       })
     );
   }

toolkit/components/places/nsNavHistoryResult.cpp

@@ -3443,8 +3443,9 @@ nsresult nsNavHistoryFolderResultNode::OnItemVisited(nsIURI* aURI,
 nsresult nsNavHistoryFolderResultNode::OnItemMoved(
     int64_t aItemId, int32_t aOldIndex, int32_t aNewIndex, uint16_t aItemType,
     const nsACString& aGUID, const nsACString& aOldParentGUID,
-    const nsACString& aNewParentGUID, uint16_t aSource,
+    const nsACString& aNewParentGUID, uint16_t aSource, const nsACString& aURI,
-    const nsACString& aURI) {
+    const nsACString& aTitle, const nsAString& aTags, int64_t aFrecency,
+    bool aHidden, uint32_t aVisitCount, PRTime aLastVisitDate) {
   MOZ_ASSERT(aOldParentGUID.Equals(mTargetFolderGuid) ||
                  aNewParentGUID.Equals(mTargetFolderGuid),
              "Got a bookmark message that doesn't belong to us");
@@ -3506,26 +3507,11 @@ nsresult nsNavHistoryFolderResultNode::OnItemMoved(
                   aGUID, aOldParentGUID, aSource);
   }
   if (aNewParentGUID.Equals(mTargetFolderGuid)) {
-    nsAutoCString title;
-    nsAutoString tags;
-    int64_t frecency = 0;
-    bool hidden = false;
-    uint32_t visitCount = 0;
-    PRTime lastVisitDate = 0;
-    if (node) {
-      title.Assign(node->mTitle);
-      tags.Assign(node->mTags);
-      frecency = node->mFrecency;
-      hidden = node->mHidden;
-      visitCount = node->mAccessCount;
-      lastVisitDate = node->mTime;
-    }
-
     OnItemAdded(
         aItemId, mTargetFolderItemId, aNewIndex, aItemType, itemURI,
         RoundedPRNow(),  // This is a dummy dateAdded, not the real value.
-        aGUID, aNewParentGUID, aSource, title, tags, frecency, hidden,
+        aGUID, aNewParentGUID, aSource, aTitle, aTags, aFrecency, aHidden,
-        visitCount, lastVisitDate);
+        aVisitCount, aLastVisitDate);
   }
 
   return NS_OK;
@@ -4299,13 +4285,23 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
             item->mOldParentGuid,
             OnItemMoved(item->mId, item->mOldIndex, item->mIndex,
                         item->mItemType, item->mGuid, item->mOldParentGuid,
-                        item->mParentGuid, item->mSource, url));
+                        item->mParentGuid, item->mSource, url,
+                        NS_ConvertUTF16toUTF8(item->mTitle), item->mTags,
+                        item->mFrecency, item->mHidden, item->mVisitCount,
+                        item->mLastVisitDate.IsNull()
+                            ? 0
+                            : item->mLastVisitDate.Value() * 1000));
         if (!item->mParentGuid.Equals(item->mOldParentGuid)) {
           ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
               item->mParentGuid,
               OnItemMoved(item->mId, item->mOldIndex, item->mIndex,
                           item->mItemType, item->mGuid, item->mOldParentGuid,
-                          item->mParentGuid, item->mSource, url));
+                          item->mParentGuid, item->mSource, url,
+                          NS_ConvertUTF16toUTF8(item->mTitle), item->mTags,
+                          item->mFrecency, item->mHidden, item->mVisitCount,
+                          item->mLastVisitDate.IsNull()
+                              ? 0
+                              : item->mLastVisitDate.Value() * 1000));
         }
         ENUMERATE_ALL_BOOKMARKS_OBSERVERS(
             OnItemMoved(item->mId, item->mOldIndex, item->mIndex,

toolkit/components/places/nsNavHistoryResult.h

@@ -803,7 +803,9 @@ class nsNavHistoryFolderResultNode final
                        uint16_t aItemType, const nsACString& aGUID,
                        const nsACString& aOldParentGUID,
                        const nsACString& aNewParentGUID, uint16_t aSource,
-                       const nsACString& aURI);
+                       const nsACString& aURI, const nsACString& aTitle,
+                       const nsAString& aTags, int64_t aFrecency, bool aHidden,
+                       uint32_t aVisitCount, PRTime aLastVisitDate);
   nsresult OnItemVisited(nsIURI* aURI, int64_t aVisitId, PRTime aTime);
 
   virtual void OnRemoving() override;

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

@@ -80,6 +80,12 @@ function expectPlacesObserverNotifications(
             oldParentGuid: event.oldParentGuid,
             oldIndex: event.oldIndex,
             isTagging: event.isTagging,
+            title: event.title,
+            tags: event.tags,
+            frecency: event.frecency,
+            hidden: event.hidden,
+            visitCount: event.visitCount,
+            lastVisitDate: event.lastVisitDate,
           });
           break;
         case "bookmark-tags-changed":

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

@@ -539,6 +539,30 @@ add_task(async function bookmarkItemMoved_bookmark() {
               Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
           },
           { name: "url", check: v => typeof v == "string" },
+          {
+            name: "title",
+            check: v => v == bm.title,
+          },
+          {
+            name: "tags",
+            check: v => v === "",
+          },
+          {
+            name: "frecency",
+            check: v => v === 1,
+          },
+          {
+            name: "hidden",
+            check: v => v === false,
+          },
+          {
+            name: "visitCount",
+            check: v => v === 0,
+          },
+          {
+            name: "lastVisitDate",
+            check: v => v === null,
+          },
         ],
       },
       {
@@ -569,6 +593,30 @@ add_task(async function bookmarkItemMoved_bookmark() {
               Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
           },
           { name: "url", check: v => typeof v == "string" },
+          {
+            name: "title",
+            check: v => v == bm.title,
+          },
+          {
+            name: "tags",
+            check: v => v === "",
+          },
+          {
+            name: "frecency",
+            check: v => v === 1,
+          },
+          {
+            name: "hidden",
+            check: v => v === false,
+          },
+          {
+            name: "visitCount",
+            check: v => v === 0,
+          },
+          {
+            name: "lastVisitDate",
+            check: v => v === null,
+          },
         ],
       },
     ]),

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

@@ -266,6 +266,12 @@ async function testMoveToFolder(details) {
         oldParentGuid: origItem.parentGuid,
         oldIndex: notification.originalIndex,
         isTagging: false,
+        title: origItem.title,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       });
     }
     observer.check(expectedNotifications);

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

@@ -327,6 +327,12 @@ add_task(async function update_move_same_folder() {
       oldParentGuid: bm.parentGuid,
       oldIndex: bmOldIndex,
       isTagging: false,
+      title: bm.title,
+      tags: "",
+      frecency: 1,
+      hidden: false,
+      visitCount: 0,
+      lastVisitDate: null,
     },
   ]);
 
@@ -352,6 +358,12 @@ add_task(async function update_move_same_folder() {
       oldParentGuid: bm.parentGuid,
       oldIndex: bmOldIndex,
       isTagging: false,
+      title: bm.title,
+      tags: "",
+      frecency: 1,
+      hidden: false,
+      visitCount: 0,
+      lastVisitDate: null,
     },
   ]);
 });
@@ -389,6 +401,12 @@ add_task(async function update_move_different_folder() {
       oldParentGuid: PlacesUtils.bookmarks.unfiledGuid,
       oldIndex: bmOldIndex,
       isTagging: false,
+      title: bm.title,
+      tags: "",
+      frecency: 1,
+      hidden: false,
+      visitCount: 0,
+      lastVisitDate: null,
     },
   ]);
 });
@@ -427,6 +445,12 @@ add_task(async function update_move_tag_folder() {
       oldParentGuid: PlacesUtils.bookmarks.unfiledGuid,
       oldIndex: bmOldIndex,
       isTagging: true,
+      title: bm.title,
+      tags: "tag",
+      frecency: 1,
+      hidden: false,
+      visitCount: 0,
+      lastVisitDate: null,
     },
   ]);
 });
@@ -1059,6 +1083,12 @@ add_task(async function reorder_notification() {
       oldParentGuid: child.parentGuid,
       oldIndex: child.index,
       isTagging: false,
+      title: child.title,
+      tags: "",
+      frecency: child.url ? 1 : 0,
+      hidden: false,
+      visitCount: 0,
+      lastVisitDate: null,
     });
   }
 

toolkit/components/places/tests/sync/head_sync.js

@@ -310,9 +310,14 @@ BookmarkObserver.prototype = {
             guid: event.guid,
             parentGuid: event.parentGuid,
             source: event.source,
+            tags: event.tags,
+            frecency: event.frecency,
+            hidden: event.hidden,
+            visitCount: event.visitCount,
           };
           if (!this.ignoreDates) {
             params.dateAdded = event.dateAdded;
+            params.lastVisitDate = event.lastVisitDate;
           }
           this.notifications.push({ name: "bookmark-added", params });
           break;
@@ -356,6 +361,12 @@ BookmarkObserver.prototype = {
             oldIndex: event.oldIndex,
             oldParentGuid: event.oldParentGuid,
             isTagging: event.isTagging,
+            title: event.title,
+            tags: event.tags,
+            frecency: event.frecency,
+            hidden: event.hidden,
+            visitCount: event.visitCount,
+            lastVisitDate: event.lastVisitDate,
           };
           this.notifications.push({ name: "bookmark-moved", params });
           break;

toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js

@@ -466,6 +466,10 @@ add_task(async function test_apply_then_revert() {
         guid: "bookmarkFFFF",
         parentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -481,6 +485,12 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
         isTagging: false,
+        title: "E",
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -496,6 +506,12 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "",
         isTagging: false,
+        title: "A (remote)",
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -511,6 +527,12 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c",
         isTagging: false,
+        title: "C",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -526,6 +548,12 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b-remote",
         isTagging: false,
+        title: "B",
+        tags: "",
+        frecency: -1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {

toolkit/components/places/tests/sync/test_bookmark_structure_changes.js

@@ -161,6 +161,12 @@ add_task(async function test_value_structure_conflict() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
         isTagging: false,
+        title: "E",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -176,6 +182,12 @@ add_task(async function test_value_structure_conflict() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
         isTagging: false,
+        title: "B",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -406,6 +418,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "",
         isTagging: false,
+        title: "Dev",
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -421,6 +439,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "",
         isTagging: false,
+        title: "Mozilla",
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -436,6 +460,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://bugzilla.mozilla.org/",
         isTagging: false,
+        title: "Bugzilla",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -451,6 +481,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://webmaker.org/",
         isTagging: false,
+        title: "Webmaker",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -466,6 +502,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://nightly.mozilla.org/",
         isTagging: false,
+        title: "Nightly",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -481,6 +523,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://developer.mozilla.org/",
         isTagging: false,
+        title: "MDN",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -496,6 +544,12 @@ add_task(async function test_move() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://getfirefox.com/",
         isTagging: false,
+        title: "Get Firefox!",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
   ]);
@@ -715,6 +769,10 @@ add_task(async function test_move_into_parent_sibling() {
         guid: "folderCCCCCC",
         parentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -730,6 +788,12 @@ add_task(async function test_move_into_parent_sibling() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
         isTagging: false,
+        title: "B",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
   ]);
@@ -933,6 +997,10 @@ add_task(async function test_complex_move_with_additions() {
         guid: "bookmarkEEEE",
         parentGuid: "folderAAAAAA",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -948,6 +1016,12 @@ add_task(async function test_complex_move_with_additions() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c",
         isTagging: false,
+        title: "C",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -963,6 +1037,12 @@ add_task(async function test_complex_move_with_additions() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "",
         isTagging: false,
+        title: "A",
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
   ]);

toolkit/components/places/tests/sync/test_bookmark_value_changes.js

@@ -179,6 +179,11 @@ add_task(async function test_value_combo() {
         parentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         dateAdded: now,
+        tags: "taggy,browsers",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -194,6 +199,11 @@ add_task(async function test_value_combo() {
         parentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         dateAdded: now,
+        tags: "",
+        frecency: 0,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -209,6 +219,11 @@ add_task(async function test_value_combo() {
         parentGuid: "tFolder_____",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         dateAdded: now,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -224,6 +239,12 @@ add_task(async function test_value_combo() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://bugzilla.mozilla.org/",
         isTagging: false,
+        title: "Bugzilla",
+        tags: "new,tag",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -1322,6 +1343,10 @@ add_task(async function test_keywords_complex() {
         guid: "bookmarkAAAA",
         parentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -1336,6 +1361,10 @@ add_task(async function test_keywords_complex() {
         guid: "bookmarkAAA1",
         parentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -1350,6 +1379,10 @@ add_task(async function test_keywords_complex() {
         guid: "bookmarkBBB1",
         parentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
       },
     },
     {
@@ -1370,6 +1403,12 @@ add_task(async function test_keywords_complex() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
         isTagging: false,
+        title: "B",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -1385,6 +1424,12 @@ add_task(async function test_keywords_complex() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c-remote",
         isTagging: false,
+        title: "C (remote)",
+        tags: "",
+        frecency: -1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -1400,6 +1445,12 @@ add_task(async function test_keywords_complex() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/d",
         isTagging: false,
+        title: "D",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {
@@ -1415,6 +1466,12 @@ add_task(async function test_keywords_complex() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
         isTagging: false,
+        title: "E",
+        tags: "",
+        frecency: 1,
+        hidden: false,
+        visitCount: 0,
+        lastVisitDate: null,
       },
     },
     {