Closed Bug 1511700 Opened 7 years ago Closed 6 years ago

Replace nsINavBookmarksService::onItemRemoved with "bookmark-removed"

Categories

(Toolkit :: Places, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: Siddhant085, Assigned: Siddhant085, Mentored)

References

Details

Attachments

(1 file)

The new notification system (PlacesObservers::NotifyListeners) has been introduced which is expected to give performance benefits. Need to port the older events and event listeners to use this interface. This bug is to port the use of onItemRemoved (event listeners) and change nsNavBookmarks::RemoveItem to use the new notification system.
Assignee: nobody → dpsrkp.sid
Status: NEW → ASSIGNED
Blocks: 1473530
Component: Migration → Places
Product: Firefox → Toolkit
Priority: -- → P2

Siddhant: a minor change, please could we call this "bookmark-removed" for now.

Summary: Replace nsINavBookmarksService::onItemRemoved with "bookmark-item-removed" → Replace nsINavBookmarksService::onItemRemoved with "bookmark-removed"

Phasing out the old notification system for OnItemRemoved events.

Blocks: 1607245
Blocks: 1607248
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1bdfd7c37b02 Use the new notification system (PlacesObserver) for bookmark removed notifications. r=Standard8,mak
Backout by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4a8205cd7765 Backed out changeset 1bdfd7c37b02 for causing newtab node test failure CLOSED TREE

Siddhant, you should be able to run the failing tests by:

$ cd browser/components/newtab # in the source directory
$ npm install
$ npm run testmc:unit

Let us know if you need help.

(In reply to Mark Banner (:standard8) from comment #6)

$ npm install

npm ci is probably a better command here, but does basically the same thing.

I have fixed the tests and updated my changes.
https://phabricator.services.mozilla.com/D17753

Thanks, those changes look good. I've just re-triggered the landing.

Flags: needinfo?(siddhant.gupta085)
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7cff781137b5 Use the new notification system (PlacesObserver) for bookmark removed notifications. r=Standard8,mak
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

It has landed! Thank you Siddhant for all your work on this, it is great to finally get this in.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: