Closed Bug 854288 Opened 7 years ago Closed 7 years ago

Remove PlaceUtils.restoreBookmarksFromJSONFile in PlacesUtils.jsm

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: raymondlee, Assigned: andreshm)

References

Details

Attachments

(1 file, 4 obsolete files)

Remove PlaceUtils.restoreBookmarksFromJSONFile and Places.restoreBookmarksFromJSONString after bug 852034 and bug 852040 have landed.
Blocks: 854761
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → andres
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #729412 - Attachment is obsolete: true
Attached patch Patch v3 (obsolete) — Splinter Review
Attachment #729415 - Attachment is obsolete: true
Attachment #730864 - Flags: review?(mak77)
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
(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)
Attached patch v4 (obsolete) — Splinter Review
(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)
Status: NEW → ASSIGNED
Attachment #734401 - Flags: review?(mak77) → review+
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fcbecda98fb2
Status: ASSIGNED → NEW
Try is green :)
Keywords: checkin-needed
I think this patch would require the patch for bug 852034 to land first.
(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 :)
Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=9cd97a5560e8
Keywords: checkin-needed
Attachment #734401 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/8a2549ffefae
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.