User tests: Successful: Unsuccessful:
Internet Explorer older than 10 does not accept
Content-Type: application/json
and would start to download response.
Also header Content-Disposition
would force IE to download response.
It would only occur if you would try to upload file by ajax in IE < 10,
because IE 7 - 9 do not support uploading file with XHR.
To test it see a test module in Code Tracker:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_id=8103&tracker_item_id=32285
Title |
|
It should be correct now if I understood it well.
@piotrmocko Thanks, all is good now. Moved the tracker to RTC.
Following some discussion at the recent Bug Squash event, I think we should reconsider this PR. Yes, it works (I was one of the testers), but I'm beginning to doubt that it is the correct technical solution for the problem and even has the potential to cause us problems in the future.
The problem is that we are manipulating the Content-Type header as a workaround for IE < 10 on the basis of a test that may only incidentally detect that particular browser and which may also be tightly coupled to the Apache server. Furthermore, it may be that lying about the content type may not even be necessary and there may be a solution which works across all browsers (and other clients) without specific workaround logic.
I would be interested to see if it is possible to come up with a solution where we always send the correct Content-Type header (ie. "application/json").
I have read many articles about this issue.
The problem is that IE < 10 does not support uploading files with XHR.
As a workaround is to upload files by iframe and IE would start to download response file
because Joomla would set Content-Disposition header to force download
and Content-Type which is not recognise by IE as type for display.
There is a workaround to add some information to Windows Registry, but it is not a solution for a regular user.
Joomla has browser detection and we could use it also as another proof to change Content-Type.
As I know all major browsers, also mobile, supports HTTP_ACCEPT header which a key.
I do not think that this is related to server software. You can test it on several browser with one server and it would not work only in IE < 10
These are/were my concerns:
Does $_SERVER['HTTP_ACCEPT'] exist on non-Apache servers? It seems that it does exist on nginx at least (and will probably work on IIS although I have no means of testing it), so I'm happy to go along with that.
The code is effectively making "text/plain" the default and only sending "application/json" if the client explicitly requests it in the Accept header. This sounds like the wrong way round. The default should be to send the correct header and to only send "text/plain" as a workaround for a specific situation if that is required. For example, if a non-IE < 10 client sends "Accept: /" then I would want the server to respond with "application/json" rather than "text/plain".
Why are we sending a Content-Disposition header? This is hinting to the client that it should show the user a "save as..." dialog when actually we don't really want that (or at least we have no reason to suppose that that is the required behaviour). If we must send a Content-Disposition header (and I'm not sure we do), surely it should be "inline" rather than "attachment"?
A really, really trivial point is that it looks like we should be sending "Content-Disposition" rather than "Content-disposition". See: http://tools.ietf.org/html/rfc6266 I know that really shouldn't cause any problems because headers are supposed to be case-insensitive, but we should probably try to follow the RFCs as closely as possible.
The world would be more beautiful if Internet Explorer would not exists ;)
After my third commit, Accept header would not be a problem as if it does not exist we would get expected Content-Type: application/json.
Also this header is generated by application/browser and if server follows standards it should recognise it
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
I get your point. Here is my new solution.
Most of browsers when do ajax upload send headers: Accept: application/json, text/javascript, */*; q=0.01
X-Requested-With: XMLHttpRequest
while IE < 10 sends only: Accept: text/html, application/xhtml+xml, */*
So we could check if Accept
header contains text/html
and does not contains application/json
and only then change Content-Type
to text/plain
I think that Content-Disposition is not required by JSON response and I have no idea why it was added and there is no need to force download.
I have also corrected name of Content-Disposition in my initial PR to follow standards.
Labels |
Added:
?
|
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-08-13 01:03:51 |
@pweb: There is a codestyle issue with your PR, can you please fix that, then I can move this forward.
:10: trailing whitespace.
if (isset($_SERVER['HTTP_ACCEPT']) AND strpos($_SERVER['HTTP_ACCEPT'], 'application/json') !== false)
:27: trailing whitespace.
if ($this->_mime == 'application/json')
warning: 2 lines add whitespace errors.