Allow sanitized inline styles on oEmbed iframes

[westonruter]

Embeds from the New York Times are currently broken because the oEmbed <iframe> sets its height via an inline style attribute which is removed by Kses in wp_filter_oembed_result(). The result is an embed that appears broken due to the iframe not having sufficient height.

I believe the fix is simply to allow the style attribute on the iframe, and to allow the safecss_filter_attr() function to remove style properties that are deemed unsafe:

src/wp-includes/embed.php

@@ -879 +879 @@
                         'marginheight' => true,
                         'scrolling'    => true,
                         'title'        => true,
+                        'style'        => true,
                 ),
         );

The result of this change for NYTimes embeds is the inclusion of the CSS border:none;max-width:500px;min-width:300px;min-height:550px;width:100% on the iframe's style attribute.

[westonruter]

Note that when the style attribute is allowed by Kses, the value is then sanitized by safecss_filter_attr().

Note also that the filtered HTML is cached, so in order to see this change apply you'll need to delete the necessary oEmbed postmeta and transients. For example:

wp transient delete --all
post_id=1822
for key in $(wp post meta list $post_id --fields=meta_key --format=csv | grep _oembed_); do 
    wp post meta delete $post_id $key;
done

[westonruter]

Appearance of NYTimes embed in editor prior to patch

[westonruter]

Appearance of NYTimes embed on frontend prior to patch

[westonruter]

Appearance of NYTimes embed in editor after patch

[westonruter]

Appearance of NYTimes embed on frontend after patch

[joyously]

Do you have any ideas as to why that wasn't there before?
Was there some problem with inline styles on some embeds?

[westonruter]

I do not know, but @swissspidy likely would have an idea.

[swissspidy]

Is this related to ​https://github.com/WordPress/gutenberg/issues/13000 / the same issue?

---

I don't recall any specific issues with inline styles on oEmbed results. The style attribute wasn't whitelisted from the beginning, see ​https://github.com/swissspidy/oEmbed-API/commit/be484afdf308ec22cf3b10be868d1e5b52d2088c.

I would be careful with whitelisting inline styles as it could lead to issues with other providers. It would require some deliberate testing.

Looking at the inline style from this embedded content - border:none;max-width:500px;min-width:300px;min-height:550px;width:100% - I don't exactly see why it is absolutely necessary:

• Borders can be disabled with frameborder=0
• The width and height attributes on the iframe are whitelisted, allowing the provider to define the dimensions that way
• Themes can support responsive embeds, removing the need to manually have something like width:100% for embed iframes

---

PS. clearing the oEmbed cache should work via wp embed cache clear too.

[westonruter]

Replying to swissspidy:

Is this related to ​https://github.com/WordPress/gutenberg/issues/13000 / the same issue?

Yes, I think so.

I would be careful with whitelisting inline styles as it could lead to issues with other providers.

I don't see the problem with allowing a sanitized style attribute to be passed through. The same styles are allowed by authors in post content. To me it seems oEmbed providers should be allowed to use the same styles that authors can.

Looking at the inline style from this embedded content - border:none;max-width:500px;min-width:300px;min-height:550px;width:100% - I don't exactly see why it is absolutely necessary:

• Borders can be disabled with frameborder=0
• The width and height attributes on the iframe are whitelisted, allowing the provider to define the dimensions that way
• Themes can support responsive embeds, removing the need to manually have something like width:100% for embed iframes

The issue I think is that the oEmbed data is being provided generically independent of the platform. So the same HTML being sent from NY Times needs to work in both WordPress and any other consumer. I don't think WordPress can be too heavy-handed at restricting what markup is allowed, as long as the markup is safe (e.g. no scripts, restricted styles).

It would require some deliberate testing.

Testing needed for sure.