Bug 1556427 - Add node.title to bookmarks.onRemoved event r=Standard8,willdurand,markh

The `bookmarks.onRemoved` event should have a `title` attribute
reflecting the title of the removed bookmark. This behavior is
documented in an example at:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/bookmarks/onRemoved

That documented example was reportedly broken: https://github.com/mdn/content/issues/13052

Differential Revision: https://phabricator.services.mozilla.com/D139293
This commit is contained in:
Rob Wu 2022-03-30 12:57:28 +00:00
parent af5844e637
commit 18f7e30291
11 changed files with 64 additions and 9 deletions

View File

@ -156,6 +156,7 @@ let observer = new (class extends EventEmitter {
index: event.index,
type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(event.itemType),
url: getUrl(event.itemType, event.url),
title: event.title,
};
this.emit("removed", {

View File

@ -237,7 +237,7 @@ add_task(async function test_bookmarks() {
);
}
function checkOnRemoved(id, parentId, index, url, type = "folder") {
function checkOnRemoved(id, parentId, index, title, url, type = "folder") {
let removedData = collectedEvents.pop();
browser.test.assertEq(
"onRemoved",
@ -281,6 +281,11 @@ add_task(async function test_bookmarks() {
node.url,
"onRemoved event received the expected node url"
);
browser.test.assertEq(
title,
node.title,
"onRemoved event received the expected node title"
);
browser.test.assertEq(
type,
node.type,
@ -487,6 +492,7 @@ add_task(async function test_bookmarks() {
ourId,
bookmarkGuids.unfiledGuid,
0,
"new test title",
"http://example.com/",
"bookmark"
);
@ -1264,7 +1270,12 @@ add_task(async function test_bookmarks() {
collectedEvents.length,
"1 expected events received"
);
checkOnRemoved(createdFolderId, bookmarkGuids.unfiledGuid, 1);
checkOnRemoved(
createdFolderId,
bookmarkGuids.unfiledGuid,
1,
"Mozilla Folder"
);
return browser.bookmarks.search({}).then(searchResults => {
browser.test.assertEq(
@ -1340,6 +1351,7 @@ add_task(async function test_bookmarks() {
createdSeparatorId,
createdFolderId,
0,
"",
"data:",
"separator"
);
@ -1352,7 +1364,12 @@ add_task(async function test_bookmarks() {
collectedEvents.length,
"1 expected events received"
);
checkOnRemoved(createdFolderId, bookmarkGuids.unfiledGuid, 3);
checkOnRemoved(
createdFolderId,
bookmarkGuids.unfiledGuid,
3,
"Empty Folder"
);
return browser.test.assertRejects(
browser.bookmarks.get(createdFolderId),
@ -1448,6 +1465,7 @@ add_task(async function test_bookmarks() {
createdFolderId,
bookmarkGuids.unfiledGuid,
3,
"Empty Folder",
undefined,
"folder"
);

View File

@ -24,6 +24,7 @@ class PlacesBookmarkRemoved final : public PlacesBookmark {
event->mParentId = aInitDict.mParentId;
event->mIndex = aInitDict.mIndex;
event->mUrl = aInitDict.mUrl;
event->mTitle = aInitDict.mTitle;
event->mGuid = aInitDict.mGuid;
event->mParentGuid = aInitDict.mParentGuid;
event->mSource = aInitDict.mSource;
@ -42,9 +43,11 @@ class PlacesBookmarkRemoved final : public PlacesBookmark {
}
int32_t Index() { return mIndex; }
void GetTitle(nsString& aTitle) { aTitle = mTitle; }
bool IsDescendantRemoval() { return mIsDescendantRemoval; }
int32_t mIndex;
nsString mTitle;
bool mIsDescendantRemoval;
private:

View File

@ -224,6 +224,7 @@ dictionary PlacesBookmarkRemovedInit {
required long long parentId;
required unsigned short itemType;
required DOMString url;
required DOMString title;
required ByteString guid;
required ByteString parentGuid;
required unsigned short source;
@ -240,6 +241,11 @@ interface PlacesBookmarkRemoved : PlacesBookmark {
*/
readonly attribute long index;
/**
* The title of the the removed item.
*/
readonly attribute DOMString title;
/**
* The item is a descendant of an item whose notification has been sent out.
*/

View File

@ -1327,6 +1327,7 @@ var Bookmarks = Object.freeze({
new PlacesBookmarkRemoved({
id: item._id,
url,
title: item.title,
itemType: item.type,
parentId: item._parentId,
index: item.index,
@ -3275,6 +3276,7 @@ var removeFoldersContents = async function(db, folderGuids, options) {
new PlacesBookmarkRemoved({
id: item._id,
url,
title: item.title,
parentId: item._parentId,
index: item.index,
itemType: item.type,

View File

@ -1776,10 +1776,10 @@ async function initializeTempMirrorEntities(db) {
BEGIN
/* Record an item removed notification for the tag entry. */
INSERT INTO itemsRemoved(itemId, parentId, position, type, placeId, guid,
parentGuid, isUntagging)
parentGuid, title, isUntagging)
VALUES(OLD.tagEntryId, OLD.tagFolderId, OLD.tagEntryPosition,
OLD.tagEntryType, OLD.placeId, OLD.tagEntryGuid,
OLD.tagFolderGuid, 1);
OLD.tagFolderGuid, OLD.tag, 1);
DELETE FROM moz_bookmarks WHERE id = OLD.tagEntryId;
@ -1907,6 +1907,7 @@ async function initializeTempMirrorEntities(db) {
parentId INTEGER NOT NULL,
position INTEGER NOT NULL,
type INTEGER NOT NULL,
title TEXT NOT NULL,
placeId INTEGER,
parentGuid TEXT NOT NULL,
/* We record the original level of the removed item in the tree so that we
@ -2099,7 +2100,7 @@ class BookmarkObserverRecorder {
await this.db.execute(
`SELECT v.itemId AS id, v.parentId, v.parentGuid, v.position, v.type,
(SELECT h.url FROM moz_places h WHERE h.id = v.placeId) AS url,
v.guid, v.isUntagging
v.title, v.guid, v.isUntagging
FROM itemsRemoved v
${this.orderBy("v.level", "v.parentId", "v.position")}`,
null,
@ -2114,6 +2115,7 @@ class BookmarkObserverRecorder {
position: row.getResultByName("position"),
type: row.getResultByName("type"),
urlHref: row.getResultByName("url"),
title: row.getResultByName("title"),
guid: row.getResultByName("guid"),
parentGuid: row.getResultByName("parentGuid"),
isUntagging: row.getResultByName("isUntagging"),
@ -2387,6 +2389,7 @@ class BookmarkObserverRecorder {
parentId: info.parentId,
index: info.position,
url: info.urlHref || "",
title: info.title,
guid: info.guid,
parentGuid: info.parentGuid,
source: PlacesUtils.bookmarks.SOURCES.SYNC,

View File

@ -978,9 +978,9 @@ fn remove_local_items(
let mut observer_statement = db.prepare(format!(
"WITH
ops(guid, level) AS (VALUES {})
INSERT INTO itemsRemoved(itemId, parentId, position, type, placeId,
guid, parentGuid, level)
SELECT b.id, b.parent, b.position, b.type, b.fk,
INSERT INTO itemsRemoved(itemId, parentId, position, type, title,
placeId, guid, parentGuid, level)
SELECT b.id, b.parent, b.position, b.type, b.title, b.fk,
b.guid, p.guid, n.level
FROM ops n
JOIN moz_bookmarks b ON b.guid = n.guid

View File

@ -588,6 +588,7 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource) {
if (bookmark.type == TYPE_BOOKMARK) {
bookmarkRef->mUrl.Assign(NS_ConvertUTF8toUTF16(bookmark.url));
}
bookmarkRef->mTitle.Assign(NS_ConvertUTF8toUTF16(bookmark.title));
bookmarkRef->mGuid.Assign(bookmark.guid);
bookmarkRef->mParentGuid.Assign(bookmark.parentGuid);
bookmarkRef->mSource = aSource;
@ -754,6 +755,15 @@ nsresult nsNavBookmarks::GetDescendantChildren(
NS_ENSURE_SUCCESS(rv, rv);
}
bool isNull;
rv = stmt->GetIsNull(nsNavHistory::kGetInfoIndex_Title, &isNull);
NS_ENSURE_SUCCESS(rv, rv);
if (!isNull) {
rv =
stmt->GetUTF8String(nsNavHistory::kGetInfoIndex_Title, child.title);
NS_ENSURE_SUCCESS(rv, rv);
}
// Append item to children's array.
aFolderChildrenArray.AppendElement(child);
}
@ -907,6 +917,7 @@ nsresult nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId,
bookmark->mParentId = child.parentId;
bookmark->mIndex = child.position;
bookmark->mUrl.Assign(NS_ConvertUTF8toUTF16(child.url));
bookmark->mTitle.Assign(NS_ConvertUTF8toUTF16(child.title));
bookmark->mGuid.Assign(child.guid);
bookmark->mParentGuid.Assign(child.parentGuid);
bookmark->mSource = aSource;

View File

@ -372,6 +372,7 @@ add_task(async function bookmarkTagsChanged() {
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "url", check: v => v == uri.spec },
{ name: "title", check: v => v == "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -426,6 +427,7 @@ add_task(async function bookmarkTagsChanged() {
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "url", check: v => v === "" },
{ name: "title", check: v => v == TAG },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -551,6 +553,7 @@ add_task(async function bookmarkItemRemoved_bookmark() {
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "url", check: v => v === uri.spec },
{ name: "title", check: v => v == "New title" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -591,6 +594,7 @@ add_task(async function bookmarkItemRemoved_separator() {
check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR,
},
{ name: "url", check: v => v === "" },
{ name: "title", check: v => v == "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -631,6 +635,7 @@ add_task(async function bookmarkItemRemoved_folder() {
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "url", check: v => v === "" },
{ name: "title", check: v => v == "Folder 1" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -791,6 +796,7 @@ add_task(async function bookmarkItemRemoved_folder_recursive() {
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "url", check: v => v === uri.spec },
{ name: "title", check: v => v == BMTITLE },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -817,6 +823,7 @@ add_task(async function bookmarkItemRemoved_folder_recursive() {
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "url", check: v => v === "" },
{ name: "title", check: v => v == title },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -843,6 +850,7 @@ add_task(async function bookmarkItemRemoved_folder_recursive() {
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "url", check: v => v === uri.spec },
{ name: "title", check: v => v == BMTITLE },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -869,6 +877,7 @@ add_task(async function bookmarkItemRemoved_folder_recursive() {
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "url", check: v => v === "" },
{ name: "title", check: v => v == title },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),

View File

@ -337,6 +337,7 @@ BookmarkObserver.prototype = {
index: event.index,
type: event.itemType,
urlHref: event.url || null,
title: event.title,
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,

View File

@ -441,6 +441,7 @@ add_task(async function test_apply_then_revert() {
index: 1,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
urlHref: "http://example.com/d",
title: "D",
guid: "bookmarkDDDD",
parentGuid: PlacesUtils.bookmarks.menuGuid,
source: PlacesUtils.bookmarks.SOURCES.SYNC,