Changed the return type of json to "text/javascript" #94

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

matthusby commented Mar 25, 2012

In (older?) IE if the content type is set to application/json IE doesn't know what to do with it so it tries to download the file instead of just pass it back to the js function. Changing it to a content type that ie understands fixes the problem.

nyaray commented Mar 26, 2012

Please, don't do this.

While I see the value of supporting some of IEs quirks I can't really view this as defendable behaviour within a reasonable extent. Breaking this standard compliance ("The MIME media type for JSON text is application/json." ((emphasis added)), http://www.ietf.org/rfc/rfc4627.txt.pdf, page 6) is a sure-fire way of setting CB on the path to obscurity since it just opens a can of worms with bad hacks and "compliance fixes".

My suggestion is that you add a user agent sniff somewhere in your code path that checks if the user is running a dinosaur version of IE instead of breaking CB.

Cheers,
Emilio

EDIT: minor spelling/grammar

Contributor

evanmiller commented Mar 26, 2012

Matt -- can you use an after filter to fix the MIME type? e.g.

after_(_, {Status, Headers, Payload}) ->
case my_lib:is_user_agent_internet_explorer(Req) of
true ->
{Status, my_lib:replace_application_json_with_text_javascript(Headers), Payload};
fals ->
{Status, Headers, Payload}
end.

Contributor

matthusby commented Mar 26, 2012

Emilio: Ah yeah good call, perhaps we can look at the "Accept" header to find out if the client can accept "application/json" and then fall back to something else if it cant (text/plain)?

Evan: Thanks I will take a look at that in the short term. Any thoughts on above?

Contributor

evanmiller commented Mar 26, 2012

Matt: I would be OK with a solution that sets the MIME type based on the requester's capabilities, so long as the MIME type can still be manually overridden.

Emilio: In my experience, mature software is filled with "ugly hacks" that allow interoperability with the widest ecosystem. I prefer to have things "just work" even if it means occasionally adding a few lines of code I wish weren't necessary.

nyaray commented Mar 26, 2012

Matt: yeah, something like that would be more appropriate (in my opinion, in this particular case). I also realise I might have phrased my response to harshly/directly at the core issue, sorry if my knee-jerk came on too strong.

Evan: I understand where you're coming from with that. Since we're not dealing with a web server but a framework I guess it makes sense to make things "just work" as you're saying.

I haven't dug that far into the inner workings of CB, but I'm sure that you know where a fix for this issue "should" (:P) be introduced.

Contributor

evanmiller commented Mar 26, 2012

Emilio: Well, if you think web servers are browser-agnostic, try grepping the Nginx source code for the phrase "MSIE". I get 88 hits :-)

It looks like Matt knows his way around this part of the code so I will await an update pull request from him.

Contributor

matthusby commented Mar 26, 2012

Emilio: No worries :-)

Evan: I will try to take a look early this week. As far as knowing the code, its just enough to be dangerous ;-) but I will certainly give it a shot.

nyaray commented Mar 26, 2012

Evan: you just broke me, but I think I'll get over it, though ;P.

Matt: coolios, good luck with the patch :).

Contributor

matthusby commented Apr 9, 2012

Hey guys, sorry I haven't had a chance to look this over yet (ended up traveling last week, and starting a new job next week) so I am hoping to get around to this next weekend.

Contributor

evanmiller commented Jan 17, 2013

Any updates, @matthusby?

Contributor

matthusby commented Jan 17, 2013

Hey @evanmiller, sorry - I really have not been keeping up with CB like I should be :(

I don't have any patches for this (and no plans currently) - I just hard coded the content-type change into my fork for the application I had problems with.

Contributor

evanmiller commented Jan 17, 2013

Since this can be fixed with the after_ filter I am closing the issue. If someone cares enough to send me a patch I will consider it.

evanmiller closed this Jan 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment