Bug 1511062: Apply bookmark-moved event. r=mak

Depends on D102573

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

browser/base/content/browser-places.js

@@ -1818,7 +1818,7 @@ var BookmarkingUI = {
     if (this._hasBookmarksObserver) {
       PlacesUtils.bookmarks.removeObserver(this);
       PlacesUtils.observers.removeListener(
-        ["bookmark-added", "bookmark-removed"],
+        ["bookmark-added", "bookmark-removed", "bookmark-moved"],
         this.handlePlacesEvents
       );
     }
@@ -1869,7 +1869,7 @@ var BookmarkingUI = {
             PlacesUtils.bookmarks.addObserver(this);
             this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
             PlacesUtils.observers.addListener(
-              ["bookmark-added", "bookmark-removed"],
+              ["bookmark-added", "bookmark-removed", "bookmark-moved"],
               this.handlePlacesEvents
             );
             this._hasBookmarksObserver = true;
@@ -2221,6 +2221,22 @@ var BookmarkingUI = {
               isStarUpdateNeeded = true;
             }
           }
+          break;
+        case "bookmark-moved":
+          const hasMovedInOutOtherBookmarks =
+            ev.parentGuid === PlacesUtils.bookmarks.unfiledGuid ||
+            ev.oldParentGuid === PlacesUtils.bookmarks.unfiledGuid;
+          if (hasMovedInOutOtherBookmarks) {
+            this.maybeShowOtherBookmarksFolder();
+          }
+
+          const hasMovedInOutToolbar =
+            ev.parentGuid === PlacesUtils.bookmarks.toolbarGuid ||
+            ev.oldParentGuid === PlacesUtils.bookmarks.toolbarGuid;
+          if (hasMovedInOutToolbar) {
+            this.updateEmptyToolbarMessage();
+          }
+
           break;
       }
 

browser/components/extensions/parent/ext-bookmarks.js

@@ -165,6 +165,18 @@ let observer = new (class extends EventEmitter {
             guid: event.guid,
             info: { parentId: event.parentGuid, index: event.index, node },
           });
+          break;
+        case "bookmark-moved":
+          this.emit("moved", {
+            guid: event.guid,
+            info: {
+              parentId: event.parentGuid,
+              index: event.index,
+              oldParentId: event.oldParentGuid,
+              oldIndex: event.oldIndex,
+            },
+          });
+          break;
       }
     }
   }
@@ -220,7 +232,7 @@ const decrementListeners = () => {
   if (!listenerCount) {
     PlacesUtils.bookmarks.removeObserver(observer);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       observer.handlePlacesEvents
     );
   }
@@ -231,7 +243,7 @@ const incrementListeners = () => {
   if (listenerCount == 1) {
     PlacesUtils.bookmarks.addObserver(observer);
     PlacesUtils.observers.addListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       observer.handlePlacesEvents
     );
   }

browser/components/places/PlacesUIUtils.jsm

@@ -1174,14 +1174,26 @@ var PlacesUIUtils = {
     // This listener is for counting new bookmarks
     let placesUtilsObserversListener = events => {
       for (let event of events) {
-        if (
+        switch (event.type) {
-          event.type == "bookmark-added" &&
+          case "bookmark-added":
-          event.parentGuid == PlacesUtils.bookmarks.toolbarGuid
+            if (event.parentGuid == PlacesUtils.bookmarks.toolbarGuid) {
-        ) {
+              Services.telemetry.scalarAdd(
-          Services.telemetry.scalarAdd(
+                "browser.engagement.bookmarks_toolbar_bookmark_added",
-            "browser.engagement.bookmarks_toolbar_bookmark_added",
+                1
-            1
+              );
-          );
+            }
+            break;
+          case "bookmark-moved":
+            let hasMovedToToolbar =
+              event.parentGuid == PlacesUtils.bookmarks.toolbarGuid &&
+              event.oldParentGuid != PlacesUtils.bookmarks.toolbarGuid;
+            if (hasMovedToToolbar) {
+              Services.telemetry.scalarAdd(
+                "browser.engagement.bookmarks_toolbar_bookmark_added",
+                1
+              );
+            }
+            break;
         }
       }
     };
@@ -1212,13 +1224,13 @@ var PlacesUIUtils = {
 
     this._bookmarkToolbarTelemetryListening = true;
     PlacesUtils.observers.addListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-moved"],
       placesUtilsObserversListener
     );
     PlacesUtils.bookmarks.addObserver(placesUtilsBookmarksObserver);
     PlacesUtils.registerShutdownFunction(() => {
       PlacesUtils.observers.removeListener(
-        ["bookmark-added"],
+        ["bookmark-added", "bookmark-moved"],
         placesUtilsObserversListener
       );
       PlacesUtils.bookmarks.removeObserver(placesUtilsBookmarksObserver);

browser/components/places/content/editBookmark.js

@@ -348,6 +348,11 @@ var gEditItemOverlay = {
     // Observe changes.
     if (!this._observersAdded) {
       PlacesUtils.bookmarks.addObserver(this);
+      this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
+      PlacesUtils.observers.addListener(
+        ["bookmark-moved"],
+        this.handlePlacesEvents
+      );
       window.addEventListener("unload", this);
       this._observersAdded = true;
     }
@@ -566,6 +571,10 @@ var gEditItemOverlay = {
 
     if (this._observersAdded) {
       PlacesUtils.bookmarks.removeObserver(this);
+      PlacesUtils.observers.removeListener(
+        ["bookmark-moved"],
+        this.handlePlacesEvents
+      );
       window.removeEventListener("unload", this);
       this._observersAdded = false;
     }
@@ -1126,6 +1135,35 @@ var gEditItemOverlay = {
     }
   },
 
+  async handlePlacesEvents(events) {
+    for (const event of events) {
+      switch (event.type) {
+        case "bookmark-moved":
+          if (!this._paneInfo.isItem || this._paneInfo.itemId != event.id) {
+            return;
+          }
+
+          this._paneInfo.parentGuid = event.parentGuid;
+
+          if (
+            !this._paneInfo.visibleRows.has("folderRow") ||
+            event.parentGuid === this._folderMenuList.selectedItem.folderGuid
+          ) {
+            return;
+          }
+
+          // Just setting selectItem _does not_ trigger oncommand, so we don't
+          // recurse.
+          const bm = await PlacesUtils.bookmarks.fetch(event.parentGuid);
+          this._folderMenuList.selectedItem = this._getFolderMenuItem(
+            event.parentGuid,
+            bm.title
+          );
+          break;
+      }
+    }
+  },
+
   toggleItemCheckbox(item) {
     // Update the tags field when items are checked/unchecked in the listbox
     let tags = this._getTagsArrayFromTagsInputField();

browser/components/places/tests/browser/browser_autoshow_bookmarks_toolbar.js

@@ -165,9 +165,14 @@ add_task(async function test_move_existing_to_toolbar() {
 
       let menuList = win.document.getElementById("editBMPanel_folderMenuList");
       let itemMovedPromise = PlacesTestUtils.waitForNotification(
-        "onItemMoved",
+        "bookmark-moved",
-        (id, oldIndex, newIndex, type, guid, oldParentGuid, newParentGuid) =>
+        events =>
-          newParentGuid == PlacesUtils.bookmarks.toolbarGuid && guid == bm.guid
+          events.some(
+            e =>
+              e.parentGuid === PlacesUtils.bookmarks.toolbarGuid &&
+              e.guid === bm.guid
+          ),
+        "places"
       );
       let promisePopup = BrowserTestUtils.waitForEvent(
         menuList.menupopup,

browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js

@@ -18,9 +18,9 @@ async function openPopupAndSelectFolder(guid, newBookmark = false) {
   let notificationPromise;
   if (!newBookmark) {
     notificationPromise = PlacesTestUtils.waitForNotification(
-      "onItemMoved",
+      "bookmark-moved",
-      (id, oldIndex, newIndex, type, itemGuid, oldParentGuid, newParentGuid) =>
+      events => events.some(e => guid === e.parentGuid),
-        guid == newParentGuid
+      "places"
     );
   }
 

browser/components/places/tests/browser/browser_controller_onDrop.js

@@ -108,9 +108,12 @@ async function run_drag_test(startBookmarkIndex, insertionIndex) {
 
 add_task(async function test_simple_move_down() {
   let moveNotification = PlacesTestUtils.waitForNotification(
-    "onItemMoved",
+    "bookmark-moved",
-    (id, oldIndex, newIndex, itemType, guid, oldParentGuid, newParentGuid) =>
+    events =>
-      guid == bookmarks[0].guid && oldIndex == 0 && newIndex == 1
+      events.some(
+        e => e.guid === bookmarks[0].guid && e.oldIndex == 0 && e.index == 1
+      ),
+    "places"
   );
 
   await run_drag_test(0, 2);

browser/components/places/tests/browser/browser_default_bookmark_location.js

@@ -176,9 +176,13 @@ add_task(async function test_change_location_panel() {
   let itemGuid = win.gEditItemOverlay._paneInfo.itemGuid;
   // Make sure we wait for the move to complete.
   let itemMovedPromise = PlacesTestUtils.waitForNotification(
-    "onItemMoved",
+    "bookmark-moved",
-    (id, oldIndex, newIndex, type, guid, oldParentGuid, newParentGuid) =>
+    events =>
-      newParentGuid == PlacesUtils.bookmarks.menuGuid && guid == itemGuid
+      events.some(
+        e =>
+          e.guid === itemGuid && e.parentGuid === PlacesUtils.bookmarks.menuGuid
+      ),
+    "places"
   );
 
   // Wait for the pref to change

services/sync/modules/engines/bookmarks.js

@@ -805,7 +805,7 @@ BookmarksTracker.prototype = {
       this.handlePlacesEvents.bind(this)
     );
     PlacesUtils.observers.addListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       this._placesListener
     );
     Svc.Obs.add("bookmarks-restore-begin", this);
@@ -816,7 +816,7 @@ BookmarksTracker.prototype = {
   onStop() {
     PlacesUtils.bookmarks.removeObserver(this);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       this._placesListener
     );
     Svc.Obs.remove("bookmarks-restore-begin", this);
@@ -884,6 +884,14 @@ BookmarksTracker.prototype = {
           this._log.trace("'bookmark-removed': " + event.id);
           this._upScore();
           break;
+        case "bookmark-moved":
+          if (IGNORED_SOURCES.includes(event.source)) {
+            return;
+          }
+
+          this._log.trace("'bookmark-moved': " + event.id);
+          this._upScore();
+          break;
         case "purge-caches":
           this._log.trace("purge-caches");
           this._upScore();

toolkit/components/places/Bookmarks.jsm

@@ -49,7 +49,7 @@
  * Each successful operation notifies through the nsINavBookmarksObserver
  * interface.  To listen to such notifications you must register using
  * nsINavBookmarksService addObserver and removeObserver methods.
- * Note that bookmark addition or order changes won't notify onItemMoved for
+ * Note that bookmark addition or order changes won't notify bookmark-moved for
  * items that have their indexes changed.
  * Similarly, lastModified changes not done explicitly (like changing another
  * property) won't fire an onItemChanged notification for the lastModified
@@ -957,23 +957,11 @@ var Bookmarks = Object.freeze({
               updatedItem.source,
             ]);
           }
-          // If the item was moved, notify onItemMoved.
+          // If the item was moved, notify bookmark-moved.
           if (
             item.parentGuid != updatedItem.parentGuid ||
             item.index != updatedItem.index
           ) {
-            notify(observers, "onItemMoved", [
-              updatedItem._id,
-              item.index,
-              updatedItem.index,
-              updatedItem.type,
-              updatedItem.guid,
-              item.parentGuid,
-              updatedItem.parentGuid,
-              updatedItem.source,
-              updatedItem.url && updatedItem.url.href,
-            ]);
-
             notifications.push(
               new PlacesBookmarkMoved({
                 id: updatedItem._id,
@@ -1203,9 +1191,7 @@ var Bookmarks = Object.freeze({
 
       // Updates complete, time to notify everyone.
       for (let { updatedItem, existingItem, newParent } of updateInfos) {
-        // Notify onItemChanged to listeners.
+        // If the item was moved, notify bookmark-moved.
-        let observers = PlacesUtils.bookmarks.getObservers();
-        // If the item was moved, notify onItemMoved.
         // We use the updatedItem.index here, rather than currIndex, as the views
         // need to know where we inserted the item as opposed to where it ended
         // up.
@@ -1213,18 +1199,6 @@ var Bookmarks = Object.freeze({
           existingItem.parentGuid != updatedItem.parentGuid ||
           existingItem.index != updatedItem.index
         ) {
-          notify(observers, "onItemMoved", [
-            updatedItem._id,
-            existingItem.index,
-            updatedItem.index,
-            updatedItem.type,
-            updatedItem.guid,
-            existingItem.parentGuid,
-            updatedItem.parentGuid,
-            source,
-            existingItem.url,
-          ]);
-
           notifications.push(
             new PlacesBookmarkMoved({
               id: updatedItem._id,
@@ -1791,22 +1765,9 @@ var Bookmarks = Object.freeze({
 
       const notifications = [];
 
-      let observers = PlacesUtils.bookmarks.getObservers();
       // Note that child.index is the old index.
       for (let i = 0; i < sortedChildren.length; ++i) {
         let child = sortedChildren[i];
-        notify(observers, "onItemMoved", [
-          child._id,
-          child.index,
-          i,
-          child.type,
-          child.guid,
-          child.parentGuid,
-          child.parentGuid,
-          options.source,
-          child.url && child.url.href,
-        ]);
-
         notifications.push(
           new PlacesBookmarkMoved({
             id: child._id,

toolkit/components/places/SyncedBookmarksMirror.jsm

@@ -2061,7 +2061,6 @@ class BookmarkObserverRecorder {
     this.signal = signal;
     this.placesEvents = [];
     this.guidChangedArgs = [];
-    this.itemMovedArgs = [];
     this.itemChangedArgs = [];
     this.shouldInvalidateKeywords = false;
   }
@@ -2324,18 +2323,6 @@ class BookmarkObserverRecorder {
   }
 
   noteItemMoved(info) {
-    this.itemMovedArgs.push([
-      info.id,
-      info.oldPosition,
-      info.newPosition,
-      info.type,
-      info.guid,
-      info.oldParentGuid,
-      info.newParentGuid,
-      PlacesUtils.bookmarks.SOURCES.SYNC,
-      info.urlHref,
-    ]);
-
     this.placesEvents.push(
       new PlacesBookmarkMoved({
         id: info.id,
@@ -2432,21 +2419,6 @@ class BookmarkObserverRecorder {
       PlacesObservers.notifyListeners(this.placesEvents);
     }
 
-    await Async.yieldingForEach(
-      this.itemMovedArgs,
-      args => {
-        if (this.signal.aborted) {
-          throw new SyncedBookmarksMirror.InterruptedError(
-            "Interrupted before notifying observers for moved items"
-          );
-        }
-        this.notifyObserversWithInfo(observers, "onItemMoved", {
-          isTagging: false,
-          args,
-        });
-      },
-      yieldState
-    );
     await Async.yieldingForEach(
       this.itemChangedArgs,
       args => {

toolkit/components/places/TaggingService.jsm

@@ -22,7 +22,7 @@ function TaggingService() {
   // Observe bookmarks changes.
   PlacesUtils.bookmarks.addObserver(this);
   PlacesUtils.observers.addListener(
-    ["bookmark-added", "bookmark-removed"],
+    ["bookmark-added", "bookmark-removed", "bookmark-moved"],
     this.handlePlacesEvents
   );
 
@@ -327,7 +327,7 @@ TaggingService.prototype = {
     if (aTopic == TOPIC_SHUTDOWN) {
       PlacesUtils.bookmarks.removeObserver(this);
       PlacesUtils.observers.removeListener(
-        ["bookmark-added", "bookmark-removed"],
+        ["bookmark-added", "bookmark-removed", "bookmark-moved"],
         this.handlePlacesEvents
       );
       Services.obs.removeObserver(this, TOPIC_SHUTDOWN);
@@ -418,6 +418,15 @@ TaggingService.prototype = {
             }
           });
           break;
+        case "bookmark-moved":
+          if (
+            this._tagFolders[event.id] &&
+            PlacesUtils.bookmarks.tagsGuid === event.oldParentGuid &&
+            PlacesUtils.bookmarks.tagsGuid !== event.parentGuid
+          ) {
+            delete this._tagFolders[event.id];
+          }
+          break;
       }
     }
   },

toolkit/components/places/nsNavHistoryResult.cpp

@@ -21,6 +21,7 @@
 #include "mozilla/dom/PlacesVisit.h"
 #include "mozilla/dom/PlacesVisitRemoved.h"
 #include "mozilla/dom/PlacesVisitTitle.h"
+#include "mozilla/dom/PlacesBookmarkMoved.h"
 
 #include "nsCycleCollectionParticipant.h"
 
@@ -3488,7 +3489,7 @@ nsNavHistoryResult::~nsNavHistoryResult() {
 }
 
 void nsNavHistoryResult::StopObserving() {
-  AutoTArray<PlacesEventType, 6> events;
+  AutoTArray<PlacesEventType, 7> events;
   events.AppendElement(PlacesEventType::Favicon_changed);
   if (mIsBookmarksObserver) {
     nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
@@ -3498,6 +3499,7 @@ void nsNavHistoryResult::StopObserving() {
     }
     events.AppendElement(PlacesEventType::Bookmark_added);
     events.AppendElement(PlacesEventType::Bookmark_removed);
+    events.AppendElement(PlacesEventType::Bookmark_moved);
   }
   if (mIsMobilePrefObserver) {
     Preferences::UnregisterCallback(OnMobilePrefChangedCallback,
@@ -3618,9 +3620,10 @@ void nsNavHistoryResult::EnsureIsObservingBookmarks() {
     return;
   }
   bookmarks->AddObserver(this, true);
-  AutoTArray<PlacesEventType, 3> events;
+  AutoTArray<PlacesEventType, 4> events;
   events.AppendElement(PlacesEventType::Bookmark_added);
   events.AppendElement(PlacesEventType::Bookmark_removed);
+  events.AppendElement(PlacesEventType::Bookmark_moved);
   // If we're not observing visits yet, also add a page-visited observer to
   // serve onItemVisited.
   if (!mIsHistoryObserver && !mIsHistoryDetailsObserver) {
@@ -4186,6 +4189,36 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
             item->mGuid, item->mParentGuid, item->mSource));
         break;
       }
+      case PlacesEventType::Bookmark_moved: {
+        const dom::PlacesBookmarkMoved* item = event->AsPlacesBookmarkMoved();
+        if (NS_WARN_IF(!item)) {
+          continue;
+        }
+
+        NS_ConvertUTF16toUTF8 url(item->mUrl);
+
+        ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
+            item->mOldParentGuid,
+            OnItemMoved(item->mId, item->mOldIndex, item->mIndex,
+                        item->mItemType, item->mGuid, item->mOldParentGuid,
+                        item->mParentGuid, item->mSource, url));
+        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));
+        }
+        ENUMERATE_ALL_BOOKMARKS_OBSERVERS(
+            OnItemMoved(item->mId, item->mOldIndex, item->mIndex,
+                        item->mItemType, item->mGuid, item->mOldParentGuid,
+                        item->mParentGuid, item->mSource, url));
+        ENUMERATE_HISTORY_OBSERVERS(
+            OnItemMoved(item->mId, item->mOldIndex, item->mIndex,
+                        item->mItemType, item->mGuid, item->mOldParentGuid,
+                        item->mParentGuid, item->mSource, url));
+        break;
+      }
       case PlacesEventType::Page_title_changed: {
         const PlacesVisitTitle* titleEvent = event->AsPlacesVisitTitle();
         if (NS_WARN_IF(!titleEvent)) {

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

@@ -107,6 +107,22 @@ function expectPlacesObserverNotifications(
               });
             }
           }
+          break;
+        case "bookmark-moved":
+          notifications.push({
+            type: event.type,
+            id: event.id,
+            itemType: event.itemType,
+            url: event.url,
+            guid: event.guid,
+            parentGuid: event.parentGuid,
+            source: event.source,
+            index: event.index,
+            oldParentGuid: event.oldParentGuid,
+            oldIndex: event.oldIndex,
+            isTagging: event.isTagging,
+          });
+          break;
       }
     }
   };

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

@@ -198,7 +198,7 @@ async function testMoveToFolder(details) {
 
   let observer;
   if (details.notifications) {
-    observer = expectNotifications(true);
+    observer = expectPlacesObserverNotifications(["bookmark-moved"]);
   }
 
   let movedItems = await PlacesUtils.bookmarks.moveToFolder(
@@ -255,18 +255,17 @@ async function testMoveToFolder(details) {
       let newFolder = notification.newFolder == "folderA" ? folderA : folderB;
 
       expectedNotifications.push({
-        name: "onItemMoved",
+        type: "bookmark-moved",
-        arguments: [
+        id: await PlacesUtils.promiseItemId(origItem.guid),
-          await PlacesUtils.promiseItemId(origItem.guid),
+        itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-          notification.originalIndex,
+        url: origItem.url,
-          notification.newIndex,
+        guid: origItem.guid,
-          PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        parentGuid: newFolder.guid,
-          origItem.guid,
+        source: PlacesUtils.bookmarks.SOURCES.DEFAULT,
-          origItem.parentGuid,
+        index: notification.newIndex,
-          newFolder.guid,
+        oldParentGuid: origItem.parentGuid,
-          PlacesUtils.bookmarks.SOURCES.DEFAULT,
+        oldIndex: notification.originalIndex,
-          origItem.url,
+        isTagging: false,
-        ],
       });
     }
     observer.check(expectedNotifications);

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

@@ -328,7 +328,8 @@ add_task(async function update_move_same_folder() {
   let bmItemId = await PlacesUtils.promiseItemId(bm.guid);
   let bmOldIndex = bm.index;
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-moved"]);
+
   bm = await PlacesUtils.bookmarks.update({
     guid: bm.guid,
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
@@ -337,24 +338,23 @@ add_task(async function update_move_same_folder() {
   Assert.equal(bm.index, 0);
   observer.check([
     {
-      name: "onItemMoved",
+      type: "bookmark-moved",
-      arguments: [
+      id: bmItemId,
-        bmItemId,
+      itemType: bm.type,
-        bmOldIndex,
+      url: "http://move.example.com/",
-        bm.index,
+      guid: bm.guid,
-        bm.type,
+      parentGuid: bm.parentGuid,
-        bm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm.parentGuid,
+      index: bm.index,
-        bm.parentGuid,
+      oldParentGuid: bm.parentGuid,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      oldIndex: bmOldIndex,
-        "http://move.example.com/",
+      isTagging: false,
-      ],
     },
   ]);
 
   // Test that we get the right index for DEFAULT_INDEX input.
   bmOldIndex = 0;
-  observer = expectNotifications();
+  observer = expectPlacesObserverNotifications(["bookmark-moved"]);
   bm = await PlacesUtils.bookmarks.update({
     guid: bm.guid,
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
@@ -363,18 +363,17 @@ add_task(async function update_move_same_folder() {
   Assert.ok(bm.index > 0);
   observer.check([
     {
-      name: "onItemMoved",
+      type: "bookmark-moved",
-      arguments: [
+      id: bmItemId,
-        bmItemId,
+      itemType: bm.type,
-        bmOldIndex,
+      url: bm.url,
-        bm.index,
+      guid: bm.guid,
-        bm.type,
+      parentGuid: bm.parentGuid,
-        bm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm.parentGuid,
+      index: bm.index,
-        bm.parentGuid,
+      oldParentGuid: bm.parentGuid,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      oldIndex: bmOldIndex,
-        bm.url,
+      isTagging: false,
-      ],
     },
   ]);
 });
@@ -392,7 +391,7 @@ add_task(async function update_move_different_folder() {
   let bmItemId = await PlacesUtils.promiseItemId(bm.guid);
   let bmOldIndex = bm.index;
 
-  let observer = expectNotifications();
+  const observer = expectPlacesObserverNotifications(["bookmark-moved"]);
   bm = await PlacesUtils.bookmarks.update({
     guid: bm.guid,
     parentGuid: folder.guid,
@@ -401,18 +400,55 @@ add_task(async function update_move_different_folder() {
   Assert.equal(bm.index, 0);
   observer.check([
     {
-      name: "onItemMoved",
+      type: "bookmark-moved",
-      arguments: [
+      id: bmItemId,
-        bmItemId,
+      itemType: bm.type,
-        bmOldIndex,
+      url: "http://move.example.com/",
-        bm.index,
+      guid: bm.guid,
-        bm.type,
+      parentGuid: bm.parentGuid,
-        bm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        PlacesUtils.bookmarks.unfiledGuid,
+      index: bm.index,
-        bm.parentGuid,
+      oldParentGuid: PlacesUtils.bookmarks.unfiledGuid,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      oldIndex: bmOldIndex,
-        "http://move.example.com/",
+      isTagging: false,
-      ],
+    },
+  ]);
+});
+
+add_task(async function update_move_tag_folder() {
+  let bm = await PlacesUtils.bookmarks.insert({
+    type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: new URL("http://move.example.com/"),
+  });
+  let folder = await PlacesUtils.bookmarks.insert({
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    parentGuid: PlacesUtils.bookmarks.tagsGuid,
+    title: "tag",
+  });
+  let bmItemId = await PlacesUtils.promiseItemId(bm.guid);
+  let bmOldIndex = bm.index;
+
+  const observer = expectPlacesObserverNotifications(["bookmark-moved"]);
+  bm = await PlacesUtils.bookmarks.update({
+    guid: bm.guid,
+    parentGuid: folder.guid,
+    index: PlacesUtils.bookmarks.DEFAULT_INDEX,
+  });
+  Assert.equal(bm.index, 0);
+  observer.check([
+    {
+      type: "bookmark-moved",
+      id: bmItemId,
+      itemType: bm.type,
+      url: "http://move.example.com/",
+      guid: bm.guid,
+      parentGuid: bm.parentGuid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      index: bm.index,
+      oldParentGuid: PlacesUtils.bookmarks.unfiledGuid,
+      oldIndex: bmOldIndex,
+      isTagging: true,
     },
   ]);
 });
@@ -876,7 +912,7 @@ add_task(async function reorder_notification() {
   // Randomly reorder the array.
   sorted.sort(() => 0.5 - Math.random());
 
-  let observer = expectNotifications();
+  const observer = expectPlacesObserverNotifications(["bookmark-moved"]);
   await PlacesUtils.bookmarks.reorder(
     PlacesUtils.bookmarks.unfiledGuid,
     sorted.map(bm => bm.guid)
@@ -887,20 +923,20 @@ add_task(async function reorder_notification() {
     let child = sorted[i];
     let childId = await PlacesUtils.promiseItemId(child.guid);
     expectedNotifications.push({
-      name: "onItemMoved",
+      type: "bookmark-moved",
-      arguments: [
+      id: childId,
-        childId,
+      itemType: child.type,
-        child.index,
+      url: child.url || "",
-        i,
+      guid: child.guid,
-        child.type,
+      parentGuid: child.parentGuid,
-        child.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        child.parentGuid,
+      index: i,
-        child.parentGuid,
+      oldParentGuid: child.parentGuid,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      oldIndex: child.index,
-        child.url,
+      isTagging: false,
-      ],
     });
   }
+
   observer.check(expectedNotifications);
 });
 

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

@@ -127,11 +127,11 @@ add_task(async function setup() {
     gBookmarkSkipObserver
   );
   PlacesUtils.observers.addListener(
-    ["bookmark-added", "bookmark-removed"],
+    ["bookmark-added", "bookmark-removed", "bookmark-moved"],
     gBookmarksObserver.handlePlacesEvents
   );
   PlacesUtils.observers.addListener(
-    ["bookmark-added", "bookmark-removed"],
+    ["bookmark-added", "bookmark-removed", "bookmark-moved"],
     gBookmarkSkipObserver.handlePlacesEvents
   );
 });
@@ -492,20 +492,20 @@ add_task(async function onItemChanged_tags_bookmark() {
   await promise;
 });
 
-add_task(async function onItemMoved_bookmark() {
+add_task(async function bookmarkItemMoved_bookmark() {
   let bm = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     index: 0,
   });
   let promise = Promise.all([
-    gBookmarkSkipObserver.setup(["onItemMoved", "onItemMoved"]),
+    gBookmarkSkipObserver.setup(["bookmark-moved", "bookmark-moved"]),
     gBookmarksObserver.setup([
       {
-        name: "onItemMoved",
+        eventType: "bookmark-moved",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "oldIndex", check: v => v === 0 },
-          { name: "newIndex", check: v => v === 0 },
+          { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
@@ -519,7 +519,7 @@ add_task(async function onItemMoved_bookmark() {
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
           },
           {
-            name: "newParentGuid",
+            name: "parentGuid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
           },
           {
@@ -531,11 +531,11 @@ add_task(async function onItemMoved_bookmark() {
         ],
       },
       {
-        name: "onItemMoved",
+        eventType: "bookmark-moved",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "oldIndex", check: v => v === 0 },
-          { name: "newIndex", check: v => v === 0 },
+          { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
@@ -549,7 +549,7 @@ add_task(async function onItemMoved_bookmark() {
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
           },
           {
-            name: "newParentGuid",
+            name: "parentGuid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
           },
           {

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

@@ -344,6 +344,22 @@ BookmarkObserver.prototype = {
           this.notifications.push({ name: "bookmark-removed", params });
           break;
         }
+        case "bookmark-moved": {
+          const params = {
+            itemId: event.id,
+            type: event.itemType,
+            urlHref: event.url,
+            source: event.source,
+            guid: event.guid,
+            newIndex: event.index,
+            newParentGuid: event.parentGuid,
+            oldIndex: event.oldIndex,
+            oldParentGuid: event.oldParentGuid,
+            isTagging: event.isTagging,
+          };
+          this.notifications.push({ name: "bookmark-moved", params });
+          break;
+        }
       }
     }
   },
@@ -409,7 +425,7 @@ BookmarkObserver.prototype = {
   check(expectedNotifications) {
     PlacesUtils.bookmarks.removeObserver(this);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       this.handlePlacesEvents
     );
     if (!ObjectUtils.deepEqual(this.notifications, expectedNotifications)) {
@@ -427,7 +443,7 @@ function expectBookmarkChangeNotifications(options) {
   let observer = new BookmarkObserver(options);
   PlacesUtils.bookmarks.addObserver(observer);
   PlacesUtils.observers.addListener(
-    ["bookmark-added", "bookmark-removed"],
+    ["bookmark-added", "bookmark-removed", "bookmark-moved"],
     observer.handlePlacesEvents
   );
   return observer;

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

@@ -476,7 +476,7 @@ add_task(async function test_apply_then_revert() {
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
         oldIndex: 2,
@@ -487,10 +487,11 @@ add_task(async function test_apply_then_revert() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("folderAAAAAA"),
         oldIndex: 0,
@@ -500,11 +501,12 @@ add_task(async function test_apply_then_revert() {
         oldParentGuid: PlacesUtils.bookmarks.menuGuid,
         newParentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
-        urlHref: null,
+        urlHref: "",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
         oldIndex: 1,
@@ -515,10 +517,11 @@ add_task(async function test_apply_then_revert() {
         newParentGuid: "folderAAAAAA",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
         oldIndex: 0,
@@ -529,6 +532,7 @@ add_task(async function test_apply_then_revert() {
         newParentGuid: "folderAAAAAA",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b-remote",
+        isTagging: false,
       },
     },
     {

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

@@ -149,7 +149,7 @@ add_task(async function test_value_structure_conflict() {
   ]);
   observer.check([
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
         oldIndex: 1,
@@ -160,10 +160,11 @@ add_task(async function test_value_structure_conflict() {
         newParentGuid: "folderDDDDDD",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
         oldIndex: 0,
@@ -174,6 +175,7 @@ add_task(async function test_value_structure_conflict() {
         newParentGuid: "folderDDDDDD",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
+        isTagging: false,
       },
     },
     {
@@ -398,7 +400,7 @@ add_task(async function test_move() {
   ]);
   observer.check([
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("devFolder___"),
         oldIndex: 0,
@@ -408,11 +410,12 @@ add_task(async function test_move() {
         oldParentGuid: PlacesUtils.bookmarks.menuGuid,
         newParentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
-        urlHref: null,
+        urlHref: "",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("mozFolder___"),
         oldIndex: 1,
@@ -422,11 +425,12 @@ add_task(async function test_move() {
         oldParentGuid: "devFolder___",
         newParentGuid: PlacesUtils.bookmarks.unfiledGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
-        urlHref: null,
+        urlHref: "",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bzBmk_______"),
         oldIndex: 1,
@@ -437,10 +441,11 @@ add_task(async function test_move() {
         newParentGuid: "devFolder___",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://bugzilla.mozilla.org/",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("wmBmk_______"),
         oldIndex: 2,
@@ -451,10 +456,11 @@ add_task(async function test_move() {
         newParentGuid: "devFolder___",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://webmaker.org/",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("nightlyBmk__"),
         oldIndex: 1,
@@ -465,10 +471,11 @@ add_task(async function test_move() {
         newParentGuid: "mozFolder___",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://nightly.mozilla.org/",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("mdnBmk______"),
         oldIndex: 0,
@@ -479,10 +486,11 @@ add_task(async function test_move() {
         newParentGuid: "mozFolder___",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://developer.mozilla.org/",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("fxBmk_______"),
         oldIndex: 0,
@@ -493,6 +501,7 @@ add_task(async function test_move() {
         newParentGuid: "mozFolder___",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://getfirefox.com/",
+        isTagging: false,
       },
     },
   ]);
@@ -714,7 +723,7 @@ add_task(async function test_move_into_parent_sibling() {
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
         oldIndex: 0,
@@ -725,6 +734,7 @@ add_task(async function test_move_into_parent_sibling() {
         newParentGuid: "folderCCCCCC",
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
+        isTagging: false,
       },
     },
   ]);
@@ -931,7 +941,7 @@ add_task(async function test_complex_move_with_additions() {
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
         oldIndex: 1,
@@ -942,10 +952,11 @@ add_task(async function test_complex_move_with_additions() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("folderAAAAAA"),
         oldIndex: 0,
@@ -955,7 +966,8 @@ add_task(async function test_complex_move_with_additions() {
         oldParentGuid: PlacesUtils.bookmarks.menuGuid,
         newParentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
-        urlHref: null,
+        urlHref: "",
+        isTagging: false,
       },
     },
   ]);

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

@@ -218,7 +218,7 @@ add_task(async function test_value_combo() {
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bzBmk_______"),
         oldIndex: 0,
@@ -229,6 +229,7 @@ add_task(async function test_value_combo() {
         newParentGuid: PlacesUtils.bookmarks.toolbarGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "https://bugzilla.mozilla.org/",
+        isTagging: false,
       },
     },
     {
@@ -1364,12 +1365,12 @@ add_task(async function test_keywords_complex() {
       },
     },
     {
-      // These `onItemMoved` notifications aren't necessary: we only moved
+      // These `bookmark-moved` notifications aren't necessary: we only moved
       // (B C D E) to accomodate (A A1 B1), and Places doesn't usually fire move
       // notifications for repositioned siblings. However, detecting and filtering
       // these out complicates `noteObserverChanges`, so, for simplicity, we
       // record and fire the extra notifications.
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
         oldIndex: 0,
@@ -1380,10 +1381,11 @@ add_task(async function test_keywords_complex() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/b",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
         oldIndex: 1,
@@ -1394,10 +1396,11 @@ add_task(async function test_keywords_complex() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/c-remote",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkDDDD"),
         oldIndex: 2,
@@ -1408,10 +1411,11 @@ add_task(async function test_keywords_complex() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/d",
+        isTagging: false,
       },
     },
     {
-      name: "onItemMoved",
+      name: "bookmark-moved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
         oldIndex: 3,
@@ -1422,6 +1426,7 @@ add_task(async function test_keywords_complex() {
         newParentGuid: PlacesUtils.bookmarks.menuGuid,
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
         urlHref: "http://example.com/e",
+        isTagging: false,
       },
     },
     {

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

@@ -58,6 +58,16 @@ var observer = {
             index: event.index,
             itemType: event.itemType,
           });
+          break;
+        case "bookmark-moved":
+          this.itemsMoved.set(event.guid, {
+            oldParentGuid: event.oldParentGuid,
+            oldIndex: event.oldIndex,
+            newParentGuid: event.parentGuid,
+            newIndex: event.index,
+            itemType: event.itemType,
+          });
+          break;
       }
     }
   },
@@ -118,13 +128,13 @@ function run_test() {
   bmsvc.addObserver(observer);
   observer.handlePlacesEvents = observer.handlePlacesEvents.bind(observer);
   obsvc.addListener(
-    ["bookmark-added", "bookmark-removed"],
+    ["bookmark-added", "bookmark-removed", "bookmark-moved"],
     observer.handlePlacesEvents
   );
   registerCleanupFunction(function() {
     bmsvc.removeObserver(observer);
     obsvc.removeListener(
-      ["bookmark-added", "bookmark-removed"],
+      ["bookmark-added", "bookmark-removed", "bookmark-moved"],
       observer.handlePlacesEvents
     );
   });