Remove PlaceUtils.restoreBookmarksFromJSONFile in PlacesUtils.jsm

RESOLVED FIXED in mozilla23

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: raymondlee, Assigned: andreshm)

Tracking

Trunk
mozilla23
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
Remove PlaceUtils.restoreBookmarksFromJSONFile and Places.restoreBookmarksFromJSONString after bug 852034 and bug 852040 have landed.
(Reporter)

Updated

6 years ago
Blocks: 854761
(Assignee)

Comment 1

6 years ago
Created attachment 729412 [details] [diff] [review]
Patch v1
Assignee: nobody → andres
(Assignee)

Comment 2

6 years ago
Created attachment 729415 [details] [diff] [review]
Patch v2
Attachment #729412 - Attachment is obsolete: true
(Assignee)

Comment 3

6 years ago
Created attachment 730864 [details] [diff] [review]
Patch v3
Attachment #729415 - Attachment is obsolete: true
Attachment #730864 - Flags: review?(mak77)
(Reporter)

Comment 4

5 years ago
Some add-ons are using the PlacesUtils.restoreBookmarksFromJSONFile so only one of them is being maintained.  We should contact the author about the changes.

https://addons.mozilla.org/en-US/firefox/addon/febe/?src=search
(Reporter)

Comment 5

5 years ago
(In reply to Raymond Lee [:raymondlee] from comment #4)
> Some add-ons are using the PlacesUtils.restoreBookmarksFromJSONFile so only
> one of them is being maintained.  We should contact the author about the
> changes.
> 
https://addons.mozilla.org/en-US/firefox/addon/febe/?src=search
/2109/chrome/content/febe.js
line 2674 -- PlacesUtils.restoreBookmarksFromJSONFile(dFile); 
line 2692 -- PlacesUtils.restoreBookmarksFromJSONFile(dFile); 

https://addons.mozilla.org/en-US/firefox/addon/library-in-sidebar/?src=search
/7387/chrome/content/places.js
line 509 -- PlacesUtils.restoreBookmarksFromJSONFile(aFile);

https://addons.mozilla.org/en-US/thunderbird/addon/wat-webapplicationtab/?src=search
/55713/chrome/content/places/places.js
line 488 -- PlacesUtils.restoreBookmarksFromJSONFile(aFile);

https://addons.mozilla.org/en-US/firefox/addon/febe-mod/?src=search
/320100/chrome/content/febe.js
line 1927 -- PlacesUtils.restoreBookmarksFromJSONFile(dFile); 
line 1945 -- PlacesUtils.restoreBookmarksFromJSONFile(dFile);
Removing restoreBookmarksFromJSONString is fine since there is no consumers.

For restoreBookmarksFromJSONFile I'd say for now we may just forward the call internally to the new API with a Deprecated.warning message (similar to the others we have in PlacesUtils iirc).
Comment on attachment 730864 [details] [diff] [review]
Patch v3

please ping mano for the dependency review, let me know if that delays too much.

Clearing per previous comment, please address it, regardless we can't take this until dependencies are done.
Attachment #730864 - Flags: review?(mak77)
(Reporter)

Comment 8

5 years ago
Created attachment 734401 [details] [diff] [review]
v4

(In reply to Marco Bonardo [:mak] from comment #6)
> Removing restoreBookmarksFromJSONString is fine since there is no consumers.
> 
> For restoreBookmarksFromJSONFile I'd say for now we may just forward the
> call internally to the new API with a Deprecated.warning message (similar to
> the others we have in PlacesUtils iirc).

OK, done.
Attachment #730864 - Attachment is obsolete: true
Attachment #734401 - Flags: review?(mak77)
(Reporter)

Updated

5 years ago
Status: NEW → ASSIGNED
Attachment #734401 - Flags: review?(mak77) → review+
(Assignee)

Comment 9

5 years ago
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fcbecda98fb2
Status: ASSIGNED → NEW
(Assignee)

Comment 10

5 years ago
Try is green :)
Keywords: checkin-needed
(Reporter)

Comment 13

5 years ago
I think this patch would require the patch for bug 852034 to land first.
(Reporter)

Comment 15

5 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/138ea79ea71a
> 
> We'll see :)

I ran some tests and I can confirm that we need the patch for bug 852034 before applying this :)
(Reporter)

Comment 16

5 years ago
Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=9cd97a5560e8
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Comment 17

5 years ago
Created attachment 738867 [details] [diff] [review]
Patch for check-in
Attachment #734401 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/8a2549ffefae
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.