From de295a0344b440bf37d0c7d199943ad6ec02324b Mon Sep 17 00:00:00 2001 From: Daisuke Akatsuka Date: Thu, 6 May 2021 04:24:34 +0000 Subject: [PATCH] Bug 1511062: Implement a mechanism to fire bookmark-moved event. r=mak Depends on D102572 Differential Revision: https://phabricator.services.mozilla.com/D102573 --- dom/base/PlacesBookmarkMoved.h | 61 +++++++++++++++ dom/base/PlacesEvent.h | 3 + dom/base/moz.build | 1 + dom/chrome-webidl/PlacesEvent.webidl | 43 ++++++++++- toolkit/components/places/Bookmarks.jsm | 76 ++++++++++++++++++- .../places/SyncedBookmarksMirror.jsm | 20 +++++ .../lib/environments/privileged.js | 1 + 7 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 dom/base/PlacesBookmarkMoved.h diff --git a/dom/base/PlacesBookmarkMoved.h b/dom/base/PlacesBookmarkMoved.h new file mode 100644 index 000000000000..afad925789db --- /dev/null +++ b/dom/base/PlacesBookmarkMoved.h @@ -0,0 +1,61 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_dom_PlacesBookmarkMoved_h +#define mozilla_dom_PlacesBookmarkMoved_h + +#include "mozilla/dom/PlacesBookmark.h" + +namespace mozilla { +namespace dom { +class PlacesBookmarkMoved final : public PlacesBookmark { + public: + explicit PlacesBookmarkMoved() + : PlacesBookmark(PlacesEventType::Bookmark_moved) {} + + static already_AddRefed Constructor( + const GlobalObject& aGlobal, const PlacesBookmarkMovedInit& aInitDict) { + RefPtr event = new PlacesBookmarkMoved(); + event->mId = aInitDict.mId; + event->mItemType = aInitDict.mItemType; + event->mUrl = aInitDict.mUrl; + event->mGuid = aInitDict.mGuid; + event->mParentGuid = aInitDict.mParentGuid; + event->mIndex = aInitDict.mIndex; + event->mOldParentGuid = aInitDict.mOldParentGuid; + event->mOldIndex = aInitDict.mOldIndex; + event->mSource = aInitDict.mSource; + event->mIsTagging = aInitDict.mIsTagging; + return event.forget(); + } + + JSObject* WrapObject(JSContext* aCx, + JS::Handle aGivenProto) override { + return PlacesBookmarkMoved_Binding::Wrap(aCx, this, aGivenProto); + } + + const PlacesBookmarkMoved* AsPlacesBookmarkMoved() const override { + return this; + } + + int32_t Index() { return mIndex; } + int32_t OldIndex() { return mOldIndex; } + void GetOldParentGuid(nsCString& aOldParentGuid) { + aOldParentGuid = mOldParentGuid; + } + + int32_t mIndex; + int32_t mOldIndex; + nsCString mOldParentGuid; + + private: + ~PlacesBookmarkMoved() = default; +}; + +} // namespace dom +} // namespace mozilla + +#endif diff --git a/dom/base/PlacesEvent.h b/dom/base/PlacesEvent.h index 70056a27aa57..2ca287946d7c 100644 --- a/dom/base/PlacesEvent.h +++ b/dom/base/PlacesEvent.h @@ -41,6 +41,9 @@ class PlacesEvent : public nsWrapperCache { virtual const PlacesBookmarkRemoved* AsPlacesBookmarkRemoved() const { return nullptr; } + virtual const PlacesBookmarkMoved* AsPlacesBookmarkMoved() const { + return nullptr; + } virtual const PlacesFavicon* AsPlacesFavicon() const { return nullptr; } virtual const PlacesVisitTitle* AsPlacesVisitTitle() const { return nullptr; } virtual const PlacesHistoryCleared* AsPlacesHistoryCleared() const { diff --git a/dom/base/moz.build b/dom/base/moz.build index 96d9a4f2fa8c..839bb23e2a39 100644 --- a/dom/base/moz.build +++ b/dom/base/moz.build @@ -220,6 +220,7 @@ EXPORTS.mozilla.dom += [ "ParentProcessMessageManager.h", "PlacesBookmark.h", "PlacesBookmarkAddition.h", + "PlacesBookmarkMoved.h", "PlacesBookmarkRemoved.h", "PlacesEvent.h", "PlacesFavicon.h", diff --git a/dom/chrome-webidl/PlacesEvent.webidl b/dom/chrome-webidl/PlacesEvent.webidl index 3e2d0952f744..f255043b0673 100644 --- a/dom/chrome-webidl/PlacesEvent.webidl +++ b/dom/chrome-webidl/PlacesEvent.webidl @@ -12,9 +12,14 @@ enum PlacesEventType { "bookmark-added", /** * data: PlacesBookmarkRemoved. Fired whenever a bookmark - * (or a bookmark folder/separator) is created. + * (or a bookmark folder/separator) is removed. */ "bookmark-removed", + /** + * data: PlacesBookmarkMoved. Fired whenever a bookmark + * (or a bookmark folder/separator) is moved. + */ + "bookmark-moved", /** * data: PlacesFavicon. Fired whenever a favicon changes. */ @@ -217,6 +222,42 @@ interface PlacesBookmarkRemoved : PlacesBookmark { readonly attribute boolean isDescendantRemoval; }; +dictionary PlacesBookmarkMovedInit { + required long long id; + required unsigned short itemType; + DOMString? url = null; + required ByteString guid; + required ByteString parentGuid; + required long index; + required ByteString oldParentGuid; + required long oldIndex; + required unsigned short source; + required boolean isTagging; +}; + +[ChromeOnly, Exposed=Window] +/** + * NOTE: Though PlacesBookmarkMoved extends PlacesBookmark, + * parentId will not be set. Use parentGuid instead. + */ +interface PlacesBookmarkMoved : PlacesBookmark { + constructor(PlacesBookmarkMovedInit initDict); + /** + * The item's index in the folder. + */ + readonly attribute long index; + + /** + * The unique ID associated with the item's old parent. + */ + readonly attribute ByteString oldParentGuid; + + /** + * The item's old index in the folder. + */ + readonly attribute long oldIndex; +}; + dictionary PlacesFaviconInit { required DOMString url; required ByteString pageGuid; diff --git a/toolkit/components/places/Bookmarks.jsm b/toolkit/components/places/Bookmarks.jsm index bacf172666b9..73acafcfdf15 100644 --- a/toolkit/components/places/Bookmarks.jsm +++ b/toolkit/components/places/Bookmarks.jsm @@ -664,7 +664,9 @@ var Bookmarks = Object.freeze({ insertInfos[i] = Object.assign({}, item); } - PlacesObservers.notifyListeners(notifications); + if (notifications.length) { + PlacesObservers.notifyListeners(notifications); + } return insertInfos; })(); @@ -848,6 +850,8 @@ var Bookmarks = Object.freeze({ ); } + const notifications = []; + // Notify onItemChanged to listeners. let observers = PlacesUtils.bookmarks.getObservers(); // For lastModified, we only care about the original input, since we @@ -969,6 +973,27 @@ var Bookmarks = Object.freeze({ updatedItem.source, updatedItem.url && updatedItem.url.href, ]); + + notifications.push( + new PlacesBookmarkMoved({ + id: updatedItem._id, + itemType: updatedItem.type, + url: updatedItem.url && updatedItem.url.href, + guid: updatedItem.guid, + parentGuid: updatedItem.parentGuid, + source: updatedItem.source, + index: updatedItem.index, + oldParentGuid: item.parentGuid, + oldIndex: item.index, + isTagging: + updatedItem.parentGuid === Bookmarks.tagsGuid || + parent.parentGuid === Bookmarks.tagsGuid, + }) + ); + } + + if (notifications.length) { + PlacesObservers.notifyListeners(notifications); } // Remove non-enumerable properties. @@ -1133,6 +1158,7 @@ var Bookmarks = Object.freeze({ newParent, syncChangeDelta ); + info.newParent = newParent; // For items moving within the same folder, we have to keep track // of their indexes. Otherwise we run the risk of not correctly @@ -1173,8 +1199,10 @@ var Bookmarks = Object.freeze({ } ); + const notifications = []; + // Updates complete, time to notify everyone. - for (let { updatedItem, existingItem } of updateInfos) { + for (let { updatedItem, existingItem, newParent } of updateInfos) { // Notify onItemChanged to listeners. let observers = PlacesUtils.bookmarks.getObservers(); // If the item was moved, notify onItemMoved. @@ -1196,11 +1224,32 @@ var Bookmarks = Object.freeze({ source, existingItem.url, ]); + + notifications.push( + new PlacesBookmarkMoved({ + id: updatedItem._id, + itemType: updatedItem.type, + url: existingItem.url, + guid: updatedItem.guid, + parentGuid: updatedItem.parentGuid, + source, + index: updatedItem.index, + oldParentGuid: existingItem.parentGuid, + oldIndex: existingItem.index, + isTagging: + updatedItem.parentGuid === Bookmarks.tagsGuid || + newParent.parentGuid === Bookmarks.tagsGuid, + }) + ); } // Remove non-enumerable properties. delete updatedItem.source; } + if (notifications.length) { + PlacesObservers.notifyListeners(notifications); + } + return updateInfos.map(updateInfo => Object.assign({}, updateInfo.updatedItem) ); @@ -1740,6 +1789,8 @@ var Bookmarks = Object.freeze({ options ); + const notifications = []; + let observers = PlacesUtils.bookmarks.getObservers(); // Note that child.index is the old index. for (let i = 0; i < sortedChildren.length; ++i) { @@ -1755,6 +1806,27 @@ var Bookmarks = Object.freeze({ options.source, child.url && child.url.href, ]); + + notifications.push( + new PlacesBookmarkMoved({ + id: child._id, + itemType: child.type, + url: child.url && child.url.href, + guid: child.guid, + parentGuid: child.parentGuid, + source: options.source, + index: i, + oldParentGuid: child.parentGuid, + oldIndex: child.index, + isTagging: + child.parentGuid === Bookmarks.tagsGuid || + parent.parentGuid === Bookmarks.tagsGuid, + }) + ); + } + + if (notifications.length) { + PlacesObservers.notifyListeners(notifications); } })(); }, diff --git a/toolkit/components/places/SyncedBookmarksMirror.jsm b/toolkit/components/places/SyncedBookmarksMirror.jsm index 3557e0bc00a9..a3774a8c2051 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.jsm +++ b/toolkit/components/places/SyncedBookmarksMirror.jsm @@ -2206,10 +2206,12 @@ class BookmarkObserverRecorder { await this.db.execute( `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 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 ${this.orderBy("c.level", "b.parent", "b.position")}`, null, (row, cancel) => { @@ -2226,6 +2228,7 @@ class BookmarkObserverRecorder { newPosition: row.getResultByName("newPosition"), oldPosition: row.getResultByName("oldPosition"), urlHref: row.getResultByName("url"), + grandParentGuid: row.getResultByName("grandParentGuid"), }; this.noteItemMoved(info); } @@ -2332,6 +2335,23 @@ class BookmarkObserverRecorder { PlacesUtils.bookmarks.SOURCES.SYNC, info.urlHref, ]); + + this.placesEvents.push( + new PlacesBookmarkMoved({ + id: info.id, + itemType: info.type, + url: info.urlHref, + guid: info.guid, + parentGuid: info.newParentGuid, + source: PlacesUtils.bookmarks.SOURCES.SYNC, + index: info.newPosition, + oldParentGuid: info.oldParentGuid, + oldIndex: info.oldPosition, + isTagging: + info.newParentGuid === PlacesUtils.bookmarks.tagsGuid || + info.grandParentGuid === PlacesUtils.bookmarks.tagsGuid, + }) + ); } noteItemChanged(info) { diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js index d7d30ce6cf62..9754dc346bd4 100644 --- a/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js +++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js @@ -427,6 +427,7 @@ module.exports = { Permissions: false, PlacesBookmark: false, PlacesBookmarkAddition: false, + PlacesBookmarkMoved: false, PlacesBookmarkRemoved: false, PlacesEvent: false, PlacesHistoryCleared: false,