462774 - drop JSON.jsm

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Simon Bünzli]

JSON.jsm's export was renamed from JSON to JSONModule in bug 458959. As we've now got native JSON and as consumers of JSON.jsm will have to be updated, anyway, this is the ideal moment to completely drop that module.

Comment 1

[Simon Bünzli]

Attachment: remove JSON.jsm

This patch removes JSON.jsm and its unit tests. The only remaining caller in our tree(s) will be updated in bug 407110.

Brendan: Do I need review for this patch at all?

Comment 2

[Simon Bünzli]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

Mike: This is pure code removal and would probably be a good idea to be done for Beta 2 so that extension developers don't accidentally updated their code in the wrong way (using JSONModule instead of native JSON).

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

rs=me, asking sayrer to r+ since his name is most on the hg log (not that that means much! Gavin, feel free to fwd to sayrer, but this looks ok to me and I agree we want it out ASAP).

/be

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

Sounds OK to me, assuming sayrer is on board. We should update https://developer.mozilla.org/en/JSON#Using_JSON.jsm though, and perhaps get Mark to post a followup to http://starkravingfinkle.org/blog/2008/02/extension-developers-native-json-parsing/ .

Comment 5

[Simon Bünzli]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

Robert: Any objections to getting rid of this legacy code (even though native JSON doesn't allow for blacklists yet)?

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

a? after r+ please :)

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

I agree with Simon that we should get this in for beta 2 - better sooner than later.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

a=beltzner

Comment 9

[Dave Townsend [:mossop]]

Pushed: http://hg.mozilla.org/mozilla-central/rev/ec9a1864d1fb

Comment 10

[Dave Townsend [:mossop]]

I've backed this out for now as it causes the mac boxes to burn. When re-landing you'll need someone from the build team to clobber some part of the objdir to get rid of the symlink to JSON.jsm

Comment 11

[Robert Kaiser]

Don't we have to remove that file from packaging as well (i.e. packages-static and probably even adding to removed-files.in)?

Comment 12

[Robert Kaiser]

sorry, in packages-static we package modules/* but removed-files.in may still be something we want/need so auto-update removes it.

Comment 13

[Dave Townsend [:mossop]]

Good point. Looks like missing files are just ignored there so it didn't break the builds but should be done for completeness.

Comment 14

[Dave Townsend [:mossop]]

Possibly when build are cleaning up after this they can also remove the symlinks for PlacesBackground.jsm on the win and linux boxes too *sigh*

Comment 15

[Simon Bünzli]

Attachment: add JSON.jsm to removed-files.in

Comment 16

[Dave Townsend [:mossop]]

Pushed again: http://hg.mozilla.org/mozilla-central/rev/f901ad15838d

Comment 17

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
rv:1.9.1b3pre) Gecko/20090123 Shiretoko/3.1b3pre
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090123 Minefield/3.2a1pre

JSON.jsm is no longer present.