Status

()

Core
General
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Simon Bünzli, Assigned: Simon Bünzli)

Tracking

({addon-compat, dev-doc-complete, verified1.9.1})

Trunk
mozilla1.9.1b2
addon-compat, dev-doc-complete, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

10 years ago
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.
Flags: blocking1.9.1?
(Assignee)

Comment 1

10 years ago
Created attachment 345970 [details] [diff] [review]
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?
Attachment #345970 - Flags: superreview?(brendan)
(Assignee)

Updated

10 years ago
Blocks: 450633
Keywords: dev-doc-needed
(Assignee)

Comment 2

10 years ago
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).
Attachment #345970 - Flags: review?(shaver)

Updated

10 years ago
Attachment #345970 - Flags: superreview?(brendan)
Attachment #345970 - Flags: superreview+
Attachment #345970 - Flags: review?(shaver)
Attachment #345970 - Flags: review?(gavin.sharp)
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 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/ .
Attachment #345970 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 5

10 years ago
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)?
Attachment #345970 - Flags: review?(sayrer)
Attachment #345970 - Flags: approval1.9.1b2?
Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

a? after r+ please :)
Attachment #345970 - Flags: approval1.9.1b2?

Updated

10 years ago
Attachment #345970 - Flags: review?(sayrer) → review+
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.
Attachment #345970 - Flags: approval1.9.1b2?
Comment on attachment 345970 [details] [diff] [review]
remove JSON.jsm

a=beltzner
Attachment #345970 - Flags: approval1.9.1b2? → approval1.9.1b2+
Flags: blocking1.9.1? → blocking1.9.1+
Target Milestone: --- → mozilla1.9.1
Keywords: checkin-needed
Pushed: http://hg.mozilla.org/mozilla-central/rev/ec9a1864d1fb
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

10 years ago
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

10 years ago
sorry, in packages-static we package modules/* but removed-files.in may still be something we want/need so auto-update removes it.
Good point. Looks like missing files are just ignored there so it didn't break the builds but should be done for completeness.
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*
(Assignee)

Comment 15

10 years ago
Created attachment 348195 [details] [diff] [review]
add JSON.jsm to removed-files.in

Updated

10 years ago
Keywords: late-compat
Pushed again: http://hg.mozilla.org/mozilla-central/rev/f901ad15838d
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b2
Keywords: fixed1.9.1

Comment 17

10 years ago
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1

Updated

10 years ago
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.