854288 - Remove PlaceUtils.restoreBookmarksFromJSONFile in PlacesUtils.jsm

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Raymond Lee [:raymondlee]]

Remove PlaceUtils.restoreBookmarksFromJSONFile and Places.restoreBookmarksFromJSONString after bug 852034 and bug 852040 have landed.

Comment 1

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Comment 2

[Andres Hernandez [:andreshm]]

Attachment: Patch v2

Comment 3

[Andres Hernandez [:andreshm]]

Attachment: Patch v3

Comment 4

[Raymond Lee [:raymondlee]]

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

Comment 5

[Raymond Lee [:raymondlee]]

(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);

Comment 6

[Marco Bonardo [:mak]]

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 7

[Marco Bonardo [:mak]]

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.

Comment 8

[Raymond Lee [:raymondlee]]

Attachment: 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.

Comment 9

[Andres Hernandez [:andreshm]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fcbecda98fb2

Comment 10

[Andres Hernandez [:andreshm]]

Try is green :)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3bdc9a0259f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Unfortunately, that Try run of yours didn't test xpcshell. Backed out for failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e36cebdefe7

https://tbpl.mozilla.org/php/getParsedLog.php?id=21645052&tree=Mozilla-Inbound

Comment 13

[Raymond Lee [:raymondlee]]

I think this patch would require the patch for bug 852034 to land first.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/138ea79ea71a

We'll see :)

Comment 15

[Raymond Lee [:raymondlee]]

(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 :)

Comment 16

[Raymond Lee [:raymondlee]]

Pushed to try and waiting for results
https://tbpl.mozilla.org/?tree=Try&rev=9cd97a5560e8

Comment 17

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a2549ffefae

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8a2549ffefae