Remove PlacesUtils.setAnnotationsForItem

RESOLVED FIXED in Firefox 68

Status

()

Product:
Toolkit
Component:
Places
Type:
enhancement
Importance:
P2
normal
Status:
RESOLVED FIXED
Opened:
7 months ago
Updated:
3 months ago

People

(Reporter: standard8, Assigned: mihir17166, Mentored)

Tracking

(Blocks 1 bug)

Version:
unspecified
Target:
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(1 attachment, 2 obsolete attachments)

Bug 1509464 - Remove PlacesUtils.setAnnotationsForItem. r?Standard8
47 bytes, text/x-phabricator-request
Details | Review
Reporter

Description

•
7 months ago 
Following bug 1489409, we should be close enough to removing PlacesUtils.setAnnotationsForItem completely.

We no longer need to save annotations for transactions (in createItemsFromBookmarksTree), and we don't need to handle transactions when we're inserting a tree (via handleBookmarkItemSpecialData).
Reporter

Comment 1

•
5 months ago

Alex is going to look at this.

Assignee: nobody → jrkong.hfd
Mentor: standard8
Status: NEW → ASSIGNED
Reporter

Comment 2

•
5 months ago

Note: The relevant tests to run for these are in toolkit/components/places and browser/components/places.

Doing these should be enough (unless you spot other instances outside these directories in the code base, then just fix those & run the appropriate test):

./mach xpcshell-test toolkit/components/places
./mach mochitest toolkit/components/places
./mach xpcshell-test browser/components/places
./mach mochitest browser/components/places

test_utils_setAnnotationsForItem.js can be removed completely.

Comment 3

•
4 months ago

need to remove all of these?
https://searchfox.org/mozilla-central/search?q=PlacesUtils.setAnnotationsForItem&path=

(In reply to Mark Banner (:standard8) (afk until Friday) from comment #2)

Note: The relevant tests to run for these are in toolkit/components/places and browser/components/places.

Doing these should be enough (unless you spot other instances outside these directories in the code base, then just fix those & run the appropriate test):

./mach xpcshell-test toolkit/components/places
./mach mochitest toolkit/components/places
./mach xpcshell-test browser/components/places
./mach mochitest browser/components/places

test_utils_setAnnotationsForItem.js can be removed completely.

Flags: needinfo?(standard8)
Reporter

Comment 4

•
4 months ago

(In reply to Manish [:manishkk][Less Active until 24 Feb] from comment #3)

need to remove all of these?
https://searchfox.org/mozilla-central/search?q=PlacesUtils.setAnnotationsForItem&path=

Yes that is the case, but please leave this bug for Alex as per the assignee field. He was stalled for various reasons but is hoping to do this soon.

Flags: needinfo?(standard8)
Reporter

Comment 5

•
3 months ago

Passing over to Mihir, as Alex isn't able to work on this for a while.

Assignee: jrkong.hfd → mihir17166
Assignee

Comment 7

•
3 months ago

Depends on D24260

Attachment #9052400 - Attachment is obsolete: true
Attachment #9052389 - Attachment is obsolete: true

Comment 9

•
3 months ago 
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03162ab7285e
Remove PlacesUtils.setAnnotationsForItem. r=Standard8

Comment 10

•
3 months ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/03162ab7285e

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.