Audio/Video MCE views should use iframe sandboxes

[wonderboymusic]

• Iframes allow us to load all of the JS we want without borking Tiny MCE
• I can get rid of a ton of compat code.
• Audio and video previews via [embed]s are busted right now.

[iseulde]

How will themes style it?

[wonderboymusic]

We could collect script and style URLs for media + editor stylesheet in _wpmejsSettings or something - I am not 100% sure where I am going with this, but I got it working for audio/video shortcodes and embeds that return audio/video shortcodes (working on playlists now). If I get this to work, I can delete a bunch of hacks that check your current browser and fix some issues where YouTube is the source for video shortcodes.

I am going to do a lot of testing to see if this is a good idea or not. I am not gonna commit anything weird.

[wonderboymusic]

28905.3.diff​ loads editor stylesheets - this is all a rough draft.

[iseulde]

I've been trying to do something similar during GSoC, but for general use. One thing that won't work is changing styles based on the category or format of a post. You'll need to detect changes and reset the right classes for every iframe body etc.. Not so nice. :)

[wonderboymusic]

28905.4.diff​ is a few steps away from being really good. All audio/video/playlists are loaded in iframe sandboxes with all the styles and scripts their hearts desire. Audio styles are weird, but I working on it...

The shortcodes are parsed via AJAX, so there is no need for the parsing logic in mce-view.js. Because each document has its own window property, the players will destroy themselves when the iframes die (I will confirm this with more testing).

Because they are iframes with the scripts in scope, I can delete all of the crazy browser-specific logic for compatibility. This also fixes a bunch of issues: YouTube works as the video source, no placeholders need to be shown when Flash can't communicate with window.mejs for plugin-type media sources, and no compatibility layer is required for Playlists on front-end vs Tiny MCE-loaded.

None of the shortcode logic needs to be branched for MCE views, and no JS needs to be branched for init'ing the players on load.

Another reason this is good: eventually, it would be good for all sites to have the ability to be an oEmbed endpoint themselves. If I have a player of my band's music on my site and other people want to embed it on their site, this same iframe-type loading will be useful. Not as a sandbox, but knowing the basic logic required to render these things in an iframe.

Gonna throw some cold water on my face

[wonderboymusic]

28905.7.diff​ produces the following "After" results:

Before

Video URL = broken
Video as [embed] = broken
Video as [video] = works, not sandbox'd
Use an "unsupported" type like Windows Media = no player
YouTube as src for [video] = no player

Audio URL = broken
Audio as [embed] = broken
Audio as [audio] = works
Use an "unsupported" type like Windows Media = no player

Embed as URL = works
Embed as [embed] = works

Playlist as [playlist]= works, not sandbox'd
Playlist as [playlist type="video"] = works, not sandbox'd

After

Video URL = works
Video as [embed] = works
Video as [video] = works
Use an "unsupported" type like Windows Media = works
YouTube as src for [video] = works

Audio URL = works
Audio as [embed] = works
Audio as [audio] = works
Use an "unsupported" type like Windows Media = works

Embed as URL = works
Embed as [embed] = works

Playlist as [playlist] = works
Playlist as [playlist type="video"] = works

[wonderboymusic]

In 29176:

Make audio and video URLs/embed handlers work in <iframe>-sandbox'd MCE views.

Introduce:
get_editor_stylesheets()
wp_media_mce_styles().

See #28905.

[nacin]

Tons of green in [29176] with those functions but not sure where it is derived from. If this is meant to DRY up some stuff, where is that stuff?

[wonderboymusic]

Tons of red on the way - breaking it up into digestible pieces.

[wonderboymusic]

In 29177:

_WP_Editors::editor_settings() should make use of get_editor_stylesheets().

See #28905.

[wonderboymusic]

In 29178:

Add a new AJAX action: parse-media-shortcode. This async call will replace JS rendering of audio/video/playlist shortcodes.

See #28905.

[wonderboymusic]

In 29179:

Simplify creation of audio, video, and playlist MCE views by placing them in iframe sandboxes.

Wins:

• Eliminates duplication of code between PHP and JS
• Views can load JS without messing with TinyMCE and scope
• MEjs doesn't break when it loads a file plugin-mode. This allows any file type the MEjs supports to play in MCE views.
• YouTube now works as the source for video.
• Users can still style the views, editor stylesheets are included in these sandboxes.
• Audio and Video URLs and [embed]s are no longer broken.
• Remove the crazy compat code necessary to determine what file types play in what browser.
• Remove unneeded Underscore templates.
• Remove the compat code for playlists.

See #28905.

[iseulde]

Just wondering, but if there are hacks necessary to make it work in every browser without an iframe, why are these hacks not needed for an iframe? Sorry, I don't know anything about mejs.

Problem 2: the time "label" that overflows the player is now cut off, because you can't overflow an iframe.

[iseulde]

If you have more than one of the same audio view, only the last one will display.

[wonderboymusic]

The JS outside of the iframe is inaccessible when MEjs loads media in Flash or Silverlight. In another ticket, I got it to work, but barely and the solution was hacky. Files like Windows Media worked everywhere in every browser except when included in MCE views. Because these sandboxes load the JS with HTML, they are in the same scope if Flash is used.

[iseulde]

Okay, thanks. :) Ugh, on the front-end, none of this would be a problem.

[iseulde]

So having more than one of the same view won't work because right now it's creating an iframe and tries to insert the exact same iframe in the editor for every view... It should create a new iframe for every view.
I also see that the main body's class is copied instead of the editor's body class. We'll need the editor instance there to get the body. This will also need to update whenever you change the post format etc. There are now events for that since the editor scrolling "feature".
Let me make a patch.

[iseulde]

This will set and iframe for every node in every editor. I used the editor's body class for the iframe body class, I assume that's what we need there, instead of the main body class. I uncommented it though, since it seems to mess with the rendering of the media. I'll let you figure that out. :)

[iseulde]

Just removed some whitespace in .2.

[wonderboymusic]

In 29187:

Cleanup after [29179]:

• _WP_Editors::editor_settings() no longer needs to load MEjs styles
• Make sure each identical shortcode with multiple instances also has an iframe sandbox for each instance
• For the time being, make audio and video shortcodes bypass the loading placeholder to avoid whiplash visually

Props avryl, wonderboymusic.
See #28905.

[wonderboymusic]

In 29189:

Cleanup after [29179]:

• Cleanup players when the editor is hidden - window scope is unique to each frame
• Add the editor body class to each iframe sandbox
• Remove unneeded code from wp-mediaelement.js

See #28905.

[wonderboymusic]

In 29191:

Cleanup after [29179]:

• Pause players when media is edited via modal
• Remove players on unind
• Account for failure when an empty node is passed to an mce.view.View

See #28905.

[iseulde]

.4: Don't attach nodes to the view instance and check if the iframe still exists before resizing.

[wonderboymusic]

In 29193:

Cleanup after [29179]:

Don't attach nodes to the view instance and check if the iframe still exists before resizing.

Props avryl.
See #28905.

[wonderboymusic]

In 29194:

Cleanup after [29179]:

• Remove wp.media.mixin.pauseAllPlayers() - was only ever intended to be used in MCE views, reaches too far into global scope. Rendered irrelevant since [28364].
• Remove wp.media.mixin.pausePlayers() - added in [28364]. MCE views no longer keep track of player instances and call mixin methods, they refer to their child iframes and call MEjs methods on the iframe's window.mejs.players.

See #28905.

[wonderboymusic]

In 29198:

Make sure that the ready event only fires once for relevant MCE views that are sandbox'd in iframes. Move some editor callbacks to initialize.

Props avryl.
See #28905.

[azaozz]

This bit: var p, win = $( 'iframe', content ).get(0).contentWindow; in both pausePlayers() and unsetPlayers() breaks all external embeds that use iframe (most of them?). If the iframe is not "same origin" we can't access contentWindow.

On top of that Chrome throws Uncaught SecurityError: Blocked a frame with origin... making it impossible to use even try{} catch(e){} there.

Thinking a better solution would be to fire an editor event for pausing/removing the ME.js players. Then listen for it from inside the (local) iframes we've added. That would make it asynchronous, i.e. you don't need to select any players, don't even need to know if there are any.

Another (easy?) workaround would be to add a className to the local iframes we created and look for that before trying to access contentWindow.

[azaozz]

In 29202:

TinyMCE wpView: prevent fatal (security) errors when trying to access iframe.contentWindow in pausePlayers() and unsetPlayers(). See #28905.

[azaozz]

In 29272:

wpView: consolidate pausePlayers() and unsetPlayers(), they are almost the same. Prevent errors when instead of a player ME.js shows only a "Download File" placeholder (in IE). See #28905.

[kovshenin]

The next audio file in a playlist won't play after r29179. See 28905.9.diff​.

[wonderboymusic]

In 29277:

After [29179], remove the last instance of this.isCompatibleSrc().

Props kovshenin.
See #28905.

[iseulde]

Not sure if I should reopen this. Here's an issues caused by loading the editor stylesheet in all iframes: #29048.

[wonderboymusic]

Let's handle it over there

[azaozz]

I'm still at a loss here.

• All audio/video/playlist views seem to use wp_ajax_parse_media_shortcode(). Is the block about audio and video in wp_ajax_parse_embed() still needed (https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/ajax-actions.php#L2678).
• If yes, wp_ajax_parse_embed() and wp_ajax_parse_media_shortcode() seem to repeat quite a bit of code. Merge them? Or perhaps parse_media_shortcode() should be a helper function called from wp_ajax_parse_embed()?

[wonderboymusic]

The [embed] shortcode/handler for audio and video files returns an [audio] or [video] via $wp_embed->autoembed() at priority 8 of the_content. The shortcode is parsed at priority 11.

wp_ajax_parse_embed() does both of these steps for audio/video, not for playlist, which has no embed handler.

The only shared code is the MCE style loading and the nuking of $wp_scripts->done

[azaozz]

Replying to wonderboymusic:

Sounds good.