Insert Media modal: IE 7, 8, and 9 issues

[SergeyBiryukov]

1. The vertical line on the left is displayed in IE 9, but not in IE 8 (see 22446.ie8.png​, 22446.ie9.png​). [22610]

2. "Drop files anywhere to upload" shouldn't be displayed in IE 7/8/9, as it doesn't support drag-and-drop (at least it doesn't on wp-admin/media-new.php). [22609]

3. IE 7 needs some attention for the gallery view, see 22446.ie7_gallery.png​

4. IE 7 doesn't display the selected images (22446.ie7_selected_images.png​) and in IE 9 you can see the second line (22446.ie9_selected_images.png​)

5. In IE 7 and IE 8 you can't see which image is currently selected (blue border is missing), see 22446.ie8_selected_image.png​. The check mark has been improved. Should be enough.

6. In IE 7 Embed from URL doesn't work. Nothing happens after inserting an URL.

7. When uploading an image in IE 7 ( and IE 8?) a pop up comes up which wants to download admin-ajax.php. The file includes the JSON respond. [22845]

8. IE 8: Upload an image, switch to media library, try to change the filter dropdown. It will open the upload dialog. [22781]

9. IE 8, 9, 10: Non-image thumbnails are quite off, see 22446.ie9.selected-music.png​​ or 22446.ie9.selected-music2.png​​.

The missing borders around the images are a result of the use of box-shadow, which IE 7 and 8 doesn't support. I think we should try to use real CSS borders here.

In 22609:

Media: Only display "Drop files" text when the browser supports drag and drop. see #22446, #21390.

In 22610:

Media: Add an explicit border to menus to show a divider in older browsers. see #22446, #21390.

In 22611:

Media: The zIndex arms race.

Adjusts zIndexes to be not-as-absurd as before, in the process moving the modal below the plupload flash shims (which do not have customizable zIndexes or classes, sadly).

fixes #22441, see #22446, #21390.

In 22612:

Media: zIndex peace was only temporary.

Revert [22611] in favor of using plupload's container setting. Turns out, plupload's flash shim is forced to the exact same index as the admin bar, so balancing the two is better solved by nesting the shim.

see #22441, #22446, #21390.

Priority 1: IE9 and IE8.

Priority 10: IE7. If we just hide this feature for IE7, I won't complain. We need to spend time on making sure that IE9 and IE8 work. Until they have no bugs, IE7 shouldn't be touched.

In IE8 When I upload an image (by clicking "Select files"), I see the following: ​http://cl.ly/image/2X392f3O2c3l (tested locally and on my MediaTemple server).

RE: comment:16:

MarkJaquith got on Skype and helped me figure out a fix for this. In wp-includes/functions.php line 2276 if you replace:

@header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) );

with:

@header( 'Content-Type: text/html;' );

The issue is resolved for me. Can we do this conditionally for IE7 & IE8?

I forgot about this. Older versions of IE don't like application/json and should instead receive text/plain (I think, over text/html). Not sure if there are workarounds beyond browser detection or always serving text/*.

I seem to recall there being an explicit JS-based, koop-ordered reason for delivering application/json instead of text/html here. He will know if text/plain would be okay instead.

Replying to lessbloat:

... Can we do this conditionally for IE7 & IE8?

Not just IE7 and 8, IE9 doesn't understand application/json either.

We're using application/json so jQuery's ajax method automatically parses the JSON. It also accepts text/javascript, which sounds like a viable IE fallback. I'd like to see a patch that sniffs for IE and replies with text/javascript instead.

If that doesn't work, we can overload it client-side to make sure that takes place, but then our shiny new ajax functions lose a bit of their luster.

Replying to koopersmith:

I'd like to see a patch that sniffs for IE and replies with text/javascript instead.

Unfortunately IE doesn't recognize text/javascript either. Only text/html.

Would that patch be:

if ( $is_IE )
  @header( 'Content-Type: text/html;' );
else
  @header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) );

Replying to azaozz:

Would that patch be:

if ( $is_IE )
  @header( 'Content-Type: text/html;' );
else
  @header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) );

$is_IE didn't appear to be set (or accessible), I borrowed most of your code for 22446-download-ajax.diff which​ fixes this issue for me in IE8.

Note: IE7 continues to output errors when attempting to upload an image. Could be issues in addition to the Content-Type declaration.

The fact that all of the other XHR requests are performing correctly means that jQuery is handling the current code correctly — the error here is with plupload. And understandably so, as it is juggling numerous transports.

Instead of using the new wp_send_json_* methods (which we already had to manually parse), we should just use the traditional method of echoing some json and calling wp_die() in wp_ajax_upload_attachment().

Attaching a patch to do just that.

+1 to 22446.upload.diff​. Works for me in IE7 and IE8.

+1 to commit 22446.upload.diff​, works here too.

In 22845:

Don't use wp_send_json_* handlers in the upload-attachment XHR handler.

The difference is the content type: application/json (which jQuery deserializes automatically for us) and the default text/html.

jQuery correctly handles application/json requests for IE, so we can continue to use the wp_send_json_* handlers elsewhere. Plupload rolls its own requests and does not handle application/json correctly. So, keep the standard text/html content type on upload-attachment.

props koopersmith.
see #22446.