Last Comment Bug 707142 - Support unprefixed responseType == "json" in XMLHttpRequest
: Support unprefixed responseType == "json" in XMLHttpRequest
Status: RESOLVED FIXED
[qa-]
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Henri Sivonen (:hsivonen)
:
Mentors:
http://dvcs.w3.org/hg/xhr/raw-file/ti...
: 708522 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-02 05:10 PST by Henri Sivonen (:hsivonen)
Modified: 2014-01-19 11:47 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
WIP: code, no tests yet (5.15 KB, patch)
2011-12-07 05:23 PST, Henri Sivonen (:hsivonen)
no flags Details | Diff | Review
Replace moz-json with spec-compliant json (13.11 KB, patch)
2011-12-07 08:17 PST, Henri Sivonen (:hsivonen)
bugs: review+
Details | Diff | Review
Backport to string-frozen branches without the console message (10.77 KB, patch)
2011-12-09 07:34 PST, Henri Sivonen (:hsivonen)
bugs: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Review

Description Henri Sivonen (:hsivonen) 2011-12-02 05:10:58 PST
The XHR spec now has a "json" responseType. We only have "moz-json".

Should add support for "json".

sicking, "moz-json" isn't even in the release channel yet. Do we need to keep supporting also "moz-json" or can we just replace it with "json"?

(It would be cool to get support for the unprefixed type into Aurora and Beta. That way, the prefixed version would never reach the release channel and cause legacy to be created.)
Comment 1 Jonas Sicking (:sicking) 2011-12-02 11:02:51 PST
I don't think we need to support moz-json no.

If you attach a patch, feel free to ask for beta/aurora approval, I don't know how likely it is to get it.
Comment 2 Henri Sivonen (:hsivonen) 2011-12-07 05:23:28 PST
Created attachment 579667 [details] [diff] [review]
WIP: code, no tests yet
Comment 3 Henri Sivonen (:hsivonen) 2011-12-07 08:17:13 PST
Created attachment 579694 [details] [diff] [review]
Replace moz-json with spec-compliant json
Comment 4 :Ms2ger 2011-12-07 11:38:53 PST
Comment on attachment 579694 [details] [diff] [review]
Replace moz-json with spec-compliant json

>--- a/content/base/src/nsXMLHttpRequest.cpp
>+++ b/content/base/src/nsXMLHttpRequest.cpp
> nsXMLHttpRequest::CreateResponseParsedJSON(JSContext* aCx)
> {
>   if (!aCx) {
>     return NS_ERROR_FAILURE;
>   }
>-
>+  // The Unicode converter has already zapped the BOM if there was one
>   if (!JS_ParseJSON(aCx,
>                     (jschar*)mResponseText.get(),

Can you make this static_cast<const jschar*> while you're here?
Comment 5 Jonas Sicking (:sicking) 2011-12-07 11:57:13 PST
I'm personally not a fan of enforcing utf8-only for JSON. I don't really see the harm in allowing other encodings when explicitly declared through http-header. Consistency with other text-based formats seems better to me.

But I'll leave that up to you guys.
Comment 6 Masatoshi Kimura [:emk] 2011-12-08 04:07:23 PST
*** Bug 708522 has been marked as a duplicate of this bug. ***
Comment 7 Henri Sivonen (:hsivonen) 2011-12-08 06:06:11 PST
(In reply to Ms2ger from comment #4)
> Comment on attachment 579694 [details] [diff] [review]
> Replace moz-json with spec-compliant json
> 
> >--- a/content/base/src/nsXMLHttpRequest.cpp
> >+++ b/content/base/src/nsXMLHttpRequest.cpp
> > nsXMLHttpRequest::CreateResponseParsedJSON(JSContext* aCx)
> > {
> >   if (!aCx) {
> >     return NS_ERROR_FAILURE;
> >   }
> >-
> >+  // The Unicode converter has already zapped the BOM if there was one
> >   if (!JS_ParseJSON(aCx,
> >                     (jschar*)mResponseText.get(),
> 
> Can you make this static_cast<const jschar*> while you're here?

Sorry, already landed based on seeing smaug's r+ in bugmail:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0f8871174a5

(In reply to Jonas Sicking (:sicking) from comment #5)
> I'm personally not a fan of enforcing utf8-only for JSON. I don't really see
> the harm in allowing other encodings when explicitly declared through
> http-header. Consistency with other text-based formats seems better to me.
> 
> But I'll leave that up to you guys.

This is really a discussion for public-webapps, but forcing authors to use UTF-8 everywhere we can (i.e. when using new features) is good, because that way there's less of a chance of authors lazily creating apps that don't deal with personal names, addresses, etc. outside their immediate imagination. (See the thread Faruk started of the WHATWG list, too.)
Comment 8 Masatoshi Kimura [:emk] 2011-12-08 12:15:36 PST
(In reply to Jonas Sicking (:sicking) from comment #1)
> I don't think we need to support moz-json no.
> 
> If you attach a patch, feel free to ask for beta/aurora approval, I don't
> know how likely it is to get it.

It would be more practical to create a patch to disable "moz-json" for aurora and beta. We don't have to be in a hurry to accumulate a legacy.
Comment 9 Jonas Sicking (:sicking) 2011-12-08 13:57:49 PST
I don't think we'll accumulate the legacy that fast. It seems more valuable to get author feedback as soon as possible.

A simple patch to just remove the "moz-" prefix but leave the throwing and charset as it is seems like it would be safe enough to take on branches.
Comment 10 Ed Morley [:emorley] 2011-12-09 07:08:10 PST
https://hg.mozilla.org/mozilla-central/rev/b0f8871174a5
Comment 11 Henri Sivonen (:hsivonen) 2011-12-09 07:34:24 PST
Created attachment 580410 [details] [diff] [review]
Backport to string-frozen branches without the console message

(In reply to Jonas Sicking (:sicking) from comment #9)
> I don't think we'll accumulate the legacy that fast. It seems more valuable
> to get author feedback as soon as possible.
> 
> A simple patch to just remove the "moz-" prefix but leave the throwing and
> charset as it is seems like it would be safe enough to take on branches.

I think as far as branch landings go, landing a backport of the real fix is as safe as disabling or merely renaming moz-json. It's extremely clear what the lines of this backport patch that wouldn't be part of a mere rename patch do.

This backport patch rebases the real fix to Aurora and removes the console message, since Aurora is string-frozen. (The patch works for Beta, too, with trivial merge conflict resolution.)
Comment 12 Henri Sivonen (:hsivonen) 2011-12-14 05:35:32 PST
Comment on attachment 580410 [details] [diff] [review]
Backport to string-frozen branches without the console message

Requesting approval for Firefox 10. The code changes here are very simple. The sooner we ship this to the release channel, the better chances we have to avoid the creation of Web content that uses the moz-prefixed version.
Comment 13 Henri Sivonen (:hsivonen) 2011-12-14 22:56:47 PST
Landed for Aurora / Firefox 10:
https://hg.mozilla.org/releases/mozilla-aurora/rev/5487edfae2de
Comment 14 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-28 13:27:47 PST
Marking qa- as I don't think this is something QA needs to verify. Please mark qa+ if this is not the case.

Note You need to log in before you can comment on or make changes to this bug.