-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Patch WordPress] Ensure block editor iframe is controlled by a service worker #668
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
adamziel
added
[Type] Bug
An existing feature does not function as intended
[Aspect] Browser
[Priority] High
[Type] UI / UX / User Experience
labels
Oct 9, 2023
adamziel
force-pushed
the
fix/service-worker-for-iframe
branch
from
October 9, 2023 08:47
2c850e7
to
47c8fb4
Compare
…ce worker Fixes #646 Patches the block editor to use a special ControlledIframe component instead of a regular HTML "iframe" element. The goal is to make the iframe use a plain HTTP URL instead of srcDoc, blob URL and other variations. Why? In Playground, the editor iframe needs to be controlled by Playground's service worker so it can serve CSS and other static assets. Otherwise all the requests originating in that iframe will yield 404s. However, different WordPress versions use a variety of iframe techniques that result in a non-controlled iframe: * 6.3 uses a binary blob URL and the frame isn't controlled by a service worker * <= 6.2 uses srcdoc had a null origin and the frame isn't controlled by a service worker * Other dynamic techniques, such as using a data URL, also fail to produce a controlled iframe HTTP URL src like src="/doc.html" seems to be the only way to create a controlled iframe. And so, this commit ensures that the block editor iframe uses a plain HTTP URL regardless of the WordPress version. Related: WordPress/gutenberg#55152
adamziel
force-pushed
the
fix/service-worker-for-iframe
branch
from
October 9, 2023 09:04
4e30afd
to
3bd1580
Compare
It works well in my testing. I'll go ahead and merge since the issue affects all Playground users. I hope I didn't miss anything, but if I did then let's promptly rollback. |
adamziel
added a commit
that referenced
this pull request
Oct 10, 2023
…ce worker (#668) ## What is this PR doing? Patches the block editor to use a special ControlledIframe component instead of a regular HTML "iframe" element. The goal is to make the iframe use a plain HTTP URL instead of srcDoc, blob URL and other variations. Normally, the patch applied here would be a huge maintenance burden over time. However, @ellatrix explores fixing the issue upstream [in the Gutenberg repo](#646). Once her PR is merged, the patch here will only be needed for a known and limited set of WordPress and Gutenberg versions and will not require ongoing reconciliation with new WP/GB releases. Fixes #646 ## What problem is it solving? In Playground, the editor iframe needs to be controlled by Playground's service worker so it can serve CSS and other static assets. Otherwise all the requests originating in that iframe will yield 404s. However, different WordPress versions use a variety of iframe techniques that result in a non-controlled iframe: * 6.3 uses a binary blob URL and the frame isn't controlled by a service worker * <= 6.2 uses srcdoc had a null origin and the frame isn't controlled by a service worker * Other dynamic techniques, such as using a data URL, also fail to produce a controlled iframe HTTP URL src like src="/doc.html" seems to be the only way to create a controlled iframe. And so, this commit ensures that the block editor iframe uses a plain HTTP URL regardless of the WordPress version. Once WordPress/gutenberg#55152 lands, this will just work in WordPress 6.4 and new Gutenberg releases. ## Testing Instructions Run `npm run dev` Then, confirm the inserter is nicely styled and there are no CSS-related 404s in the network tools. Test the following editors: * Post editor http://localhost:5400/website-server/?url=/wp-admin/post-new.php * Site editor http://localhost:5400/website-server/?url=/wp-admin/site-editor.php * For all supported WordPress versions * With and without the Gutenberg plugin (`&plugin=gutenberg`) ## Related * https://bugs.chromium.org/p/chromium/issues/detail?id=880768 * https://bugzilla.mozilla.org/show_bug.cgi?id=1293277 * w3c/ServiceWorker#765 * #42 * b7ca737
adamziel
added a commit
that referenced
this pull request
Oct 13, 2023
This PR, paired with WordPress/wordpress-develop#5481, enables previewing WordPress Pull Requests. Changes: * Adds wordpress.html – an easy entrypoint to previewing PRs * Allows pulling GH artifacts from wordpress-develop repo * Patches the controlled iframe in JavaScript, not in Docker, to enable re-applying the patch in wordpress-develop branch #668 * Serves static files using the PHP instance when they exist, not based on heuristics like path.startsWith('/wp-content')
adamziel
added a commit
that referenced
this pull request
Oct 16, 2023
This PR moves applying a WordPress patch from the Dockerfile, where WP is built into a `.data` file, to JavaScript, where it can be applied to any arbitrary WordPress installation. This is needed to patch WordPress branches in Pull Request previewer. See #700. Testing instructions: 1. Run Playground with `npm run start` 2. Go to the block editor 3. Confirm that inserter's CSS is loaded (black + icon) 4. Install the Gutenberg plugin 5. Confirm that inserter's CSS is still loaded Related to #668, #646.
adamziel
added a commit
that referenced
this pull request
Oct 16, 2023
This PR moves applying a WordPress patch from the Dockerfile, where WP is built into a `.data` file, to JavaScript, where it can be applied to any arbitrary WordPress installation. This is needed to patch WordPress branches in Pull Request previewer. See #700. Testing instructions: 1. Run Playground with `npm run start` 2. Go to the block editor 3. Confirm that inserter's CSS is loaded (black + icon) 4. Install the Gutenberg plugin 5. Confirm that inserter's CSS is still loaded Related to #668, #646.
adamziel
added a commit
that referenced
this pull request
Oct 16, 2023
…ile (#703) This PR moves applying a WordPress patch from the Dockerfile, where WP is built into a `.data` file, to JavaScript, where it can be applied to any arbitrary WordPress installation. This is needed to patch WordPress branches in Pull Request previewer. See #700. ## Testing instructions: 1. Run Playground with `npm run start` 2. Go to the block editor 3. Confirm that inserter's CSS is loaded (black + icon) 4. Install the Gutenberg plugin 5. Confirm that inserter's CSS is still loaded Related to #668, #646.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
[Aspect] Browser
[Priority] High
[Type] Bug
An existing feature does not function as intended
[Type] UI / UX / User Experience
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 is this PR doing?
Patches the block editor to use a special ControlledIframe component instead of a regular HTML "iframe" element. The goal is to make the iframe use a plain HTTP URL instead of srcDoc, blob URL and other variations.
Normally, the patch applied here would be a huge maintenance burden over time. However, @ellatrix explores fixing the issue upstream in the Gutenberg repo. Once her PR is merged, the patch here will only be needed for a known and limited set of WordPress and Gutenberg versions and will not require ongoing reconciliation with new WP/GB releases.
Fixes #646
What problem is it solving?
In Playground, the editor iframe needs to be controlled by Playground's service worker so it can serve CSS and other static assets. Otherwise all the requests originating in that iframe will yield 404s.
However, different WordPress versions use a variety of iframe techniques that result in a non-controlled iframe:
HTTP URL src like src="/doc.html" seems to be the only way to create a controlled iframe.
And so, this commit ensures that the block editor iframe uses a plain HTTP URL regardless of the WordPress version.
Once WordPress/gutenberg#55152 lands, this will just work in WordPress 6.4 and new Gutenberg releases.
Testing Instructions
Run
npm run dev
Then, confirm the inserter is nicely styled and there are no CSS-related 404s in the network tools.
Test the following editors:
&plugin=gutenberg
)Related