1316348 - Update eraseEverything in Bookmarks.jsm to notify at least the top-level folders

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P1)

Description

[Bob Silverberg [:bsilverberg]]

In bug 1221764 removeFoldersContents was updated to pass isDescendantRemoval: true to the onItemRemoved notification. The problem this introduces is that when eraseEverything calls removeFoldersContents, it will also pass that argument and any observers that are configured to ignore notifications with isDescendantRemoval: true will ignore all of the removals, including the top-level folders. The observer in the WebExtensions API is configured thusly, and therefore will ignore all notifications.

To remedy this we should emit notifications for the top-level folders (that is any folder that is a direct ancestor of menu/toolbar/unfiled) that are removed by eraseEverything.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

another possible alternative is bug 1087580, so have a single notification, instead of one for each folder in each root.
The difference is that with bug 1087580 we need to add a new notification, and hook up all the consumers to it, while with this fix we basically re-use the existing notification and consumers already know how to handle it.

This is probably "safer" short term, so I'd likely leave bug 1087580 to be done when we redo the whole notifications system in bug 1340498.

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1316348 - Make eraseEverything notify for removals within the top-level bookmarks folders to ensure correct updates on the UI.

Review commit: https://reviewboard.mozilla.org/r/171676/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171676/

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8900284 [details]
Bug 1316348 - Make eraseEverything notify for removals within the top-level bookmarks folders to ensure correct updates on the UI.

https://reviewboard.mozilla.org/r/171676/#review177450

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/556e8de56fb2
Make eraseEverything notify for removals within the top-level bookmarks folders to ensure correct updates on the UI. r=mak

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/556e8de56fb2