707142 - Support unprefixed responseType == "json" in XMLHttpRequest

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Henri Sivonen (:hsivonen)]

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) No longer reading bugmail consistently]

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)]

Attachment: WIP: code, no tests yet

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Replace moz-json with spec-compliant json

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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) No longer reading bugmail consistently]

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 7

[Henri Sivonen (:hsivonen)]

(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]]

(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) No longer reading bugmail consistently]

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]]

https://hg.mozilla.org/mozilla-central/rev/b0f8871174a5

Comment 11

[Henri Sivonen (:hsivonen)]

Attachment: 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)]

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)]

Landed for Aurora / Firefox 10:
https://hg.mozilla.org/releases/mozilla-aurora/rev/5487edfae2de

Comment 14

[u279076]

Marking qa- as I don't think this is something QA needs to verify. Please mark qa+ if this is not the case.