Ajax requests on forms with files fail on IE 9

Internet explorer is supposed to be a HTTP based HTML browser. There are a lot of examples where neither HTTP nor HTML is implemented properly.

ROFL :D

+++ b/core/lib/Drupal/Core/Controller/AjaxController.php
@@ -11,6 +11,7 @@
+use Symfony\Component\HttpFoundation\Response;

Sorry added this line accidentally.

AjaxResponse has to handle two diffrent request headers.

1. Standard XHR Requests: application/json
2. AJAX Requests from jQuery Form (Form API AJAX): text/html

jQuery Form falls back to submit via a hidden iFrame in Browser which does not support File Uploads via XHR Requests. More documentation about this is on http://malsup.com/jquery/form/#file-upload.
Maybe we should explicit check for HTTP_X_REQUESTED_WITH as described in this example?

Bump to critical because IE 9 is a fully supported browser.

In my custom applications, I have run into the same issue with IE. I resorted to using a content-type of text/plain for all of my JSON responses.

+++ b/core/lib/Drupal/Core/EventSubscriber/ContentControllerSubscriber.php
@@ -44,6 +44,7 @@ public function __construct(ContentNegotiation $negotiation) {
+    'iframeupload' => 'controller.ajax:content',

+++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php
@@ -129,20 +129,6 @@ public function onJson(GetResponseForControllerResultEvent $event) {
-  public function onIframeUpload(GetResponseForControllerResultEvent $event) {
-    $response = $event->getResponse();
-
-    // Browser IFRAMEs expect HTML. Browser extensions, such as Linkification
-    // and Skype's Browser Highlighter, convert URLs, phone numbers, etc. into
-    // links. This corrupts the JSON response. Protect the integrity of the
-    // JSON data by making it the value of a textarea.
-    // @see http://malsup.com/jquery/form/#file-upload
-    // @see http://drupal.org/node/1009382
-    $html = '<textarea>' . $response->getContent() . '</textarea>';
-
-    return new Response($html);
-  }

YAY! CONSISTENCY!

Already added some, but could use more :)

According to http://www.netmarketshare.com/browser-market-share.aspx?qprid=2&qpcustomd=0 IE9 usage is down to 9%. I'd personally be OK with treating IE9 bugs as 'major' - we'd still fix issues if they come up as opposed to won't fix, but not hold up release on it.

For administering Drupal sites, you should be able to use the latest version of a major browser. If you're not allowed to use the latest version of a supported browser to do that, you're probably not going to be allowed to use Drupal 8 anyway.

For site visitors it's up to the site builder to decide which browsers they support and which features are exposed to end users. Some sites work fine without users triggering ajax responses, some won't.

If this bug is in IE9, does it mean it's also in IE 8? I know we have already told IE 8 to get stuffed, but it has a disturbingly large presence on that pie chart...

I don't recall the last time I used IE period, much less an old version of it, so I wouldn't mind downgrading to major here.

IE8 doesn't get any JavaScript at all, so there's no AJAX request to fail.

#2390621: [policy, no patch] Update Drupal's browser support policy.

Discussed with @catch, @alexpott, @webchick, @Dries, and @effulgentsia. For now, IE9 is a supported browser, and this is actually a significant bug that site builders and users can't reasonably work around if they can't simply use a different browser (which is common for IE users). If we decide in #2390621: [policy, no patch] Update Drupal's browser support policy to change our IE9 support we will downgrade this.

I am at a Sprint right now. I am following: https://www.drupal.org/contributor-tasks/add-steps-to-reproduce

I have added "Needs steps to reproduce" tag to this issue. I will remove this tag if I can reproduce the steps and document it.

Successfully reproduced the image upload quirk using IE9 emulation in IE11. Added a Steps to reproduce section to the Issue Summary and documented the steps to reproduce there. Removed the tag: Needs steps to reproduce.

I've spent some time researching about this bug and testing
This is how the problem looks in Windows 7 - IE9 not emulated:

As webflo said, this bug is present only in forms with file uploads, in old browsers that don't support XHR file uploading. In that case, the jQuery Form plugin switches to the iframe mechanism.

Also, we have already implemented in Drupal's ajax.js file a flag ajax_iframe_upload that can be read from the PHP side.

This bug is not present in Drupal 7 because ajax_deliver takes care of setting the correct header. When Drupal 7 ajax system was removed #1677304: Remove drupal_deliver_page(), ajax_deliver(), drupal_deliver_html_page() I think this was overlooked.

So, we need to recover this important piece of code from Drupal 7's ajax_deliver.

 // Browser IFRAMEs expect HTML. With most other content types, Internet
      // Explorer presents the user with a download prompt.
      drupal_add_http_header('Content-Type', 'text/html; charset=utf-8');

This fix is not only for IE9.
From jQuery file upload wiki FAQ:

XMLHttpRequest File Uploads
The following browsers support XHR file uploads, which allows advanced usage of the file upload API:
Desktop browsers

Google Chrome
Apple Safari 5.0+
Mozilla Firefox 4.0+
Opera 12.0+
Microsoft Internet Explorer 10.0+
Mobile browsers

Apple Safari Mobile on iOS 6.0+
Google Chrome on iOS 6.0+
Google Chrome on Android 4.0+
Default Browser on Android 3.2+
Opera Mobile 12.0+

I'm not 100% sure if all these browsers need the content-type "text/html", but this is how Drupal 7 was behaving, and there wasn't problems. Also the jQuery Form FAQ suggests that IE is not the only browser.

The patch attached is a reroll of webflo's patch with a couple of changes.
.
Screenshot of IE9 response header and content, after the patch:

Please, review in the patch if it's good to add iframeupload in the file core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php

@dawehner Patch at #31 is addressing your concern

+    if ($request->get('ajax_iframe_upload', FALSE)) {

it's the same condition than in ajax_deliver of Drupal 7

The if is already wrapping the "textarea" part of the code.
Do you mean that setting "Content-Type" to "text/html" is needed for every ajax upload? and should be outside of the if ?
Mhh, I don't think so.
ajax_deliver was behaving like the patch I submitted. And the jquery form plugin also says Iframe based uploads require a Content-type of text/plain or text/html for the JSON response in the FAQ (link in #30, where I explain with more details the logic of the patch)

that code shouldn't be there, indeed. Thanks for catching that Daniel

correct interdiff. (See "reverted" in the top of the interdiff)

+++ b/core/lib/Drupal/Core/Ajax/AjaxSubscriber.php
@@ -27,6 +27,7 @@ public function onKernelRequest(GetResponseEvent $event) {
     $request->setFormat('drupal_ajax', 'application/vnd.drupal-ajax');
...
+    $request->setFormat('iframeupload', 'application/vnd.drupal-ajax');

Do we need extra docs regarding how we deal with multiple formats using the same MIME types?

Good point Wim. Or, do you recommend to create a specific mime type for the iframe upload?

With this patch, $request->getFormat('application/vnd.drupal-ajax') seems to always return the same format (just by looking at source code, not tested)

Or, do you recommend to create a specific mime type for the iframe upload?

Unless there's a reason to have it use the same MIME type, I think that'd be better, yes. Bonus from doing that: we can channel our frustration with IE in there. application/vnd.drupal-ie-lte-9-sucks-beyond-compare, for example.

That sounds like a needs work.

I find difficult to improve that mime type name

Tested manually in Windows 7 VM - IE9

Do we need that format / mime type at all? If we do, we need to set it somewhere in the JS, but I don't think we do. See attached patch, including its docs.

I don't have a Windows environment handly. @corbacho: can you test this please?

@effulgentsia. Yep, it works in IE9
And your comments in the code explains better the bug.

I think this is ready!, but I let someone else to mark it RTBC, to give more time if someone wants to argue this comment:

    // Since this affects the format of the output, it could be argued that
    // this should be implemented as a separate Accept MIME type. However,
    // that would require separate variants for each type of AJAX request
    // (e.g., drupal-ajax, drupal-dialog, drupal-modal), so for expediency,
    // this browser workaround is implemented via a GET or POST parameter.

Thanks!

Actually, I think #50 brings up good points. I have an idea for one other option for us to consider. Will try to post it today.

I await effulgentsia's alternative, but #47 looks fairly good to me.

Please note, the last patch does not work. The file upload part is fine but other ajax operations in the same form are failing. jQuery Form adds the ajax_iframe_upload flag to the file upload request only. It is not present during any other ajax operations (e.g. field widget with multiple values). IE 9 fails to send the proper accept header if a file upload field is present in the form.

so re-rtbcing an old patch without re-uploading is a bit of a mare from an automated re-test of the rtbc queue point of view. Based #10 pinted out a minor nit with #9 and in #52 @effulgentsia says he has some ideas...

@webflo: can you add a comment here with the exact Accept header and POST data that IE 9 sends during a non-file-upload AJAX operation on a form that has a file upload field?

Additionally, it would also be helpful to know the Accept header it sends during an AJAX operation on a form without any file upload fields.

I couldn't replicate the bug of @webflo

Patch works. I tried on Article content type, changing the image field to "unlimited", and also testing the autocomplete ajax.

Testing on a Virtual Machine, Windows 7, IE9

@effulgentsia, information you requested:
Default Article content type, uploading an image:
ie9-upload-POST-request.png

Default Article content type, autocomplete request (GET request have empty Request Body, obviously)
ie9-AJAX-GET-request.png

Modified Basic page content type by adding a Unlimited Text Field, clicking "Add another item" (AJAX POST request without file field in the form)
ie9-not-upload-POST-request.png
Full request body: http://pastebin.com/1U1xuF8Z

I see. It fails, the AJAX response is an HTML document :(

I added a unlimited Text field to Article content type.
Then, clicking "Add another item" makes this request/response:

ie9-non-upload-post-AJAX-with-fileupload-in-the-form.jpg

As you saw in the screenshots, the POSTs requests in IE9 has accept header "text/html" always that the form has a File Upload field

We lose our ability to identify what type of Ajax call is (drupal_modal, drupal_ajax, drupal_dialog), because all our ContentNegotiation and renderer is based on mime types.

I made a work-around (1 line in ajax.js), so we can expose the original mime type during the ContentNegotiation phase.
options.extraData.ajax_iframe_upload_mime_type = options.accepts.json + '-iframeupload';

I wish someone would have a genius idea to simplify a bit more the patch.

But this currently, works in IE9 and Chrome, and there is not much code duplication, only duplicated mime types.
I've to do more extensive testing, but first, do you agree with the approach?

needs review

@dawehner I wish too! But weblo's approach worked in *that* time, when the content negotiation and ajax rendering system was different. I tried now similar approach but does not work.

Yes, #63 patch works for all ajax requests types we have been discussing in the issue.

The bright side: this "fix" will be removed as soon as we drop IE9 support :)

Same patch than #63, with fix on test ReadTest.php

+++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php
@@ -209,7 +209,7 @@ protected function getFormat(Request $request) {
+    if (in_array($format, ['drupal_modal', 'drupal_dialog', 'drupal_ajax', 'drupal_modal_iframe', 'drupal_dialog_iframe', 'drupal_ajax_iframe', ])) {

nitpick: We don't normally add the extra ',' when the array is on one line - but - an array of over 80 chars should be split according to coding standards.

Real pity about the hardcoding/duplication here but I guess we didn't get #2331919: Implement StackNegotiation so not much we can do.

Manual tests:

Now I'm going to wash in kerosene to remove the traces of IE.

@dawehner I know that! :)
My patch worked in that scenario too, those were the steps I followed. Did you flush the aggregated JavaScript ? I wonder why didn't work for you.

.. but anyhow... I'm not wasting more time there.. (I'm not that motivated to keep killing hours in IE9).

I like more your patch, that was a smart fix to overload the "drupal_ajax" renderer, and makes everything much simpler :)

Before, every format had a renderer and mimetype associated in a 1:1:1 mapping. Not now, that's why the test failed in #71 returning "null". But I trust that this wouldn't have any further consequences.

The patch offers a fix for the "drupal_ajax" format. My only concern is what happens with drupal_dialog and drupal_modal formats. They have different renderers and arguments, do they keep working as normally when renderer is different ?

Otherwise, thumbs up!

Nitpick: I liked effulgentsia code comment at #47 explaining the bug, we could use it instead of the old one

Much cleaner than #70, addresses my whining about duplication.
Let's do it.

Let's get the comment from #47 to explain:

+++ b/core/lib/Drupal/Core/Ajax/AjaxResponse.php
@@ -115,6 +115,20 @@ public function prepareResponse(Request $request) {
+      $this->headers->set('Content-Type', 'text/html; charset=utf-8');

Also

My only concern is what happens with drupal_dialog and drupal_modal formats. They have different renderers and arguments, do they keep working as normally when renderer is different ?

What's the answer to this?

Tested in IE9 and Chrome. It works well (image uploads and also unlimited text field 'Add another item' button), well documented and that Unit Test was great great to have :)

Thumbs up!

PS. The content I put in the unlimited text (formatted) field is not saved. But this is not related to this patch, it happens also without the patch. I will open an issue ..

no, chrome too #2442379: Content of Unlimited Text Field not saved
Anyway, this is unrelated. This shouldn't stop this critical fix to be committed.

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 9c0bfb2 and pushed to 8.0.x. Thanks!

I did this:

/**
  * Implementation of hook hook_ajax_render_alter().
  */
function privatedownloadmanager_ajax_render_alter(&$commands) { // stop IE treating ajax response as a file attachment
  if(strpos($_SERVER["HTTP_USER_AGENT"], 'MSIE 9.0')>-1 || strpos($_SERVER["HTTP_USER_AGENT"], 'MSIE 8.0')>-1 || strpos($_SERVER["HTTP_USER_AGENT"], 'WOW64')>-1 || strpos($_SERVER["HTTP_USER_AGENT"], 'Trident')>-1){
    if(isset($commands[0]['settings']['urlIsAjaxTrusted'])){
      $url='';
      foreach ($commands[0]['settings']['urlIsAjaxTrusted'] as $url => $value) {
        break;
      }
      if(valid_url($url)){
        if( strpos($url, '/file/ajax')===0 || strpos($url, '/system/ajax')===0){
          drupal_add_http_header('content-type', 'text/html');
        }
      }
    }
  }
}