Embeds not working for non-WP sites

[swissspidy]

While testing #34429 @peterwilsoncc noticed that the new embeds feature doesn't work for non-WordPress sites. In fact, it seems it didn't work for quite some time, I just didn't test it properly.

The problem is that the secret shared between the iframe and the host site is added via PHP by WordPress. It should be done in the JavaScript part to ensure it works for any website.

Luckily, that's not hard to fix. The attached patch solves it in a backwards-compatible manner.

[peterwilsoncc]

Note: #34204 will require a refresh if this is committed first.

[pento]

This solution causes a race condition - if the iframe HTML loads before the parent DOMDocumentLoaded is fired, the iframe JS will try to grab the secret before its set in the URL.

If the secret hasn't been added, the iframe should sleep and retry later.

[peterwilsoncc]

iframes[ i ].src += '#?secret=' + secret; 
iframes[ i ].setAttribute( 'data-secret', secret ); 

These lines will be a problem if multiple embeds are on one page, the iframe URL will have the secret appended for each instance of the embed code. A check for data-secret before appending should fix this.

if data-secret exists, you can also continue; as that increment of the loop does not need to run.

if ( !!iframes[ i ].getAttribute( 'security' ) && (isIE10 || isIE11) ) { 

Worth reversing the attribute and IE check here, no need to look up the DOM if not IE10-11.

Could also be worth overloading the selector to iframe.wp-embedded-content to help avoid any coincidental hits.

[swissspidy]

34451.2.diff​ is an updated patch with the improvements suggested above.

• The getSecret() function in the iframe re-gets the secret when it was later on.
• Change order in the IE checks.
• Continue when data-secret attribute already exists.
• Fixes #34462 by hiding/displaying the embed content during onLoad

Note: The editor preview seems to be broken, but will be fixed by #34204

[pento]

We need to add the class in wp_filter_oembed_result(), not get_post_embed_html(). (Which reminds me - the same should be happening for the iframe.) We don't trust classes from the embedding site, as they could cause clickjacking-type side effects.

[swissspidy]

Replying to pento:

We need to add the class in wp_filter_oembed_result(), not get_post_embed_html(). (Which reminds me - the same should be happening for the iframe.) We don't trust classes from the embedding site, as they could cause clickjacking-type side effects.

Even better to add it in both places, so it works for non-WP sites. Will whip up a new patch. Anything else?

[pento]

That's all off the top of my head, but it's Friday evening, I'm all #yolofriday-ed out. :-)

[swissspidy]

34451.3.diff​ is an updated patch as per the above comment. Slightly changed the JS in wp-embed.js to correctly remove the security attribute when in Internet Explorer. Tested with IE10+.

[swissspidy]

Seems like the reason for the broken editor preview was that the secret wasn't being added to new iframes (since it only does so in onLoad.

34451.4.diff​ fixes this by not removing that part from wp_filter_oembed_result() to make the experience for both WP sites and non-WP sites the same, without breaking already existing embeds.

The only concern now would be that getSecret() could lead to an infinite loop when there's never a secret added to the iframe.

[swissspidy]

Just saw that _oembed_filter_feed_content was recently added. That string replacement needs to be changed too (and tested!) because of the added class name.

[swissspidy]

34451.5.diff​ is an updated patch that now also fixes _oembed_filter_feed_content().

@pento Can you do a final review?

[pento]

In 35577:

Embeds: Fix support for embedding in non-WordPress sites.

This moves the last of the iframe message code from PHP to JavaScript, so it can be included in any site, without needing to rely on any of WordPress' internal behaviour.

Props swissspidy.

Fixes #34451.