Iframe: always enable for block themes, in core too #66800
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -65 B (0%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
ellatrix
changed the title
ellatrix
commented
Nov 6, 2024
|
|
||
| return ( | ||
| hasV3BlocksOnly || | ||
| ( isGutenbergPlugin && isBlockBasedTheme ) || |
There was a problem hiding this comment.
I've essentially only removed isGutenbergPlugin. But I moved all this logic inside the selector to avoid calling all selectors if previous conditions are met.
ellatrix
changed the title
|
Flaky tests detected in 7da6a4b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11709095870
|
jsnajdr
approved these changes
Nov 7, 2024
|
So the only possibility for the iframe to not be used is when using a classic theme with block v2. So the next step basically is to always iframe right? |
youknowriad
added
the
Needs Dev Note
|
If we care about iframing classic themes, yes. Worth noting that we already do this when the GB plugin is enabled, this only changes things for core. |
|
What I care about is the possible perf improvements and simplifications we can make by assuming the canvas is always iframed. The fact is that we also don't test non iframed much these days either so there are potential issues and bugs we wouldn't have to deal with anymore |
|
Yeah, that's true. And we see that it can actually cause more harm than good to disable it. People have had a lot of time to update and adjust. 6.7 enabled iframe for meta boxes. 6.8 will enable it for all block based themes. 6.9 maybe for everything? It's good to take it a bit slower and give some time. |
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
We already always iframe the post editor when a block theme is active with the Gutenberg plugin active. Let's do it for core too.
It's time we move forward in enabling the iframe in more scenarios. The iframe has been widely tested over the last many years in the Site Editor, device preview modes, all block, pattern and template previews, AND the post editor whenever Gutenberg is enabled with a block theme. This basically means that the iframe is always active on WordPress.com (where the GB plugin is always active and all themes are block themes).
Why?
For greater consistency. An example complaint: https://x.com/thekevingeary/status/1803760573718397081.
We have been very cautious in enabling the iframe if there's a block that is not v3, because there's a tiny possibility it might break. In most cases this is fine though, and un-iframing the editor could break more things than it fixes.
How?
We enable the iframe for block based themes, but only when the Gutenberg plugin is active.
I propose that we always enable the iframe for block based themes, also in core. The iframe has been around for a long time and it's been widely tested.
Testing Instructions
Note that for the Gutenberg plugin, nothing has changed.
Here's a snippet for registering a v2 block:
Testing Instructions for Keyboard
Screenshots or screencast