Skip to content

Conversation

@felixarntz
Copy link
Member

wp_filter_oembed_result() checks for any "unknown" oEmbed provider whether there is a fallback blockquote element present for the iframe to load. If so, it hides the iframe visually and adds a generated data-secret attribute to the iframe and blockquote to associate them with each other. Then, the wp-embed.js script goes through the page looking for such elements. Once the iframe has been loaded (and thus notified the wp-embed.js script via postMessage), the script makes the iframe visible and hides the blockquote.

This mechanism can break when the iframe is marked to be lazy-loaded: Since the browser may interpret the hidden iframe as being irrelevant (since it's hidden), it will never load it, hence the iframe will not send the postMessage event to become visible.

This PR ensures that such iframes do not receive the loading attribute and are instead kept unmodified. It also fixes a minor issue so that the wp_img_tag_add_loading_attr and wp_iframe_tag_add_loading_attr filters no longer fire unnecessarily, and it fixes a pointless iframe test from before.

Trac ticket: https://core.trac.wordpress.org/ticket/52768


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

function test_wp_iframe_tag_add_loading_attr_opt_out() {
$iframe = '<iframe src="https://www.example.com" width="640" height="360"></iframe>';
add_filter( 'wp_iframe_tag_add_loading_attr', '__return_false' );
$iframe = wp_iframe_tag_add_loading_attr( $iframe, 'test' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call was missing before, so basically this test was testing nothing. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants