Remove iframe from base styles.

[jasmussen]

I can't recall the exact circumstances for why it was added, I suspect it had to do with embeds for themes that don't opt in to responsive embed styles. So we'll want to test with that. But this PR fixes https://core.trac.wordpress.org/ticket/48482.

This is a rule themes can override, but if it isn't necessary they shouldn't have to.

[jasmussen]

Things to test: various embeds in themes that do and do not support responsive embeds.

[aduth]

For what reason should there be any top-level element styles in this stylesheet? By the preceding comment mentioning them to be "vanilla block styles", should at the very least they be specific to blocks in some fashion? Otherwise, those styles will apply to any image, iframe, figcaption on the page (regardless of in a block or not).

[aduth]

Despite my previous comment, if this returns us to a state we were already in while avoiding future potential incompatibilities, I'd not want to block it with my concerns for applying styles to elements globally.

[aduth]

Previously: #14407 (where this was introduced, along with img, both moved from being scoped within .block-editor__container styles in edit-post/src/style.scss)

[aduth]

The iframe style was originally introduced in #995

[jasmussen]

Thanks so much for your thoughtful review and detective historical work.

As you've found, yes indeed the img and figcaption rules have been there for a while, moved there from a different stylesheet. I think what put them there was a long discussion on providing as few styles from the editor as possible, in delightful irony.

That's not to say those two styles can't also be removed, but I'm unsure of what effect that might have. Looking back at conversations, the figcaption seemed very intentional because it was such a rarely used element at the time. This can be revisited.

The img tag seems to have been out of a desire to provide baseline responsiveness out of the box.

Both img and figcaption could be reviewed again, but as you suggest, probably in a separate PR.

The iframe rule I introduced is, as has been noted on the core track ticket, probably going to cause headaches for advertisements and such, and given the dubious value it provides, might as well remove this for starters.

If you like, I can follow up with a PR for the other two rules, to address less urgently?

[aduth]

tl;dr: I think this change is safe, and in fact renders iframes more accurately than they would have ever previously been shown in the editor, dating back to #995.

Longer:

I suppose the intention with #995 was to try to better occupy the full width available in the content area for embeds in particular. However, it was only ever implemented in the editor, and was never applied to embeds on the front-end. Thus, the natural width of an embed, while not always the most ideal option (often under- or over-occupying the content width either in the editor or the front-end) is the most consistent. Based on what I expect to have been intended with #995, I think it would be worth exploring separately to apply this width specific to iframes of the Embed block, and applied consistently in the editor and on the front-end.

Aside: I had a hard time re-testing #995 because the embed preview is broken (at least for WordPress embeds) in the editor in ways neither caused nor resolved by the styles changed here.

[aduth]

While there's a larger discussion to be had about these element selectors, since the img styles were introduced at the same time as those impacting iframe (not from earlier, but as of #14407), should they too be considered for more immediate removal?

[jasmussen]

I'm not sure. I'm happy to remove it, but it wasn't introduced there, it was just moved. It used to be here: https://github.com/WordPress/gutenberg/pull/14407/files#diff-556b1933de5650dc555c469c584accc2L91

Given the hope is to get this PR into the final RC, I'm going to create a separate PR for the img styles. We can then merge and cherry pick both if need be, but at least keeping both separate concerns.

[jasmussen]

Created #18281 as a followup, and/or alternative.

[aduth]

@jasmussen As I understand it: Both img and iframe styles, from that link, were previously scoped to the block editor container. As of the changes in #14407, they were both changed to apply globally, including on the front-end, which lead to the conflicts surfaced around iframe (and which I suspect could also hold true for img).

[jasmussen]

Would you then suggest #18281 is the best solution? Or would the solution be to remove the iframe, and move the img rule back to be scoped to the editor only? Given the sense of urgency the core trac ticket has for this change I would prefer we do whatever we can to make it something we merge for 5.3.

[aduth]

I think the safest option for the short term, assuming nothing depended on the change, would be to move it back to how it was. #18281 would be the next best solution in my view.

Generally speaking, however, I do think they should both be removed / refactored to apply specifically to blocks.

[aduth]

Closed as superseded by #18287