Skip to content
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

Missing CSS and images because the Gutenberg iframe isn't controlled by the service worker #646

Open
Tracked by #653
Lovor01 opened this issue Sep 21, 2023 · 13 comments · Fixed by #668
Open
Tracked by #653
Labels
[Aspect] Service Worker [Priority] High [Type] Bug An existing feature does not function as intended [Type] Project

Comments

@Lovor01
Copy link

Lovor01 commented Sep 21, 2023

This happens only in https://playground.wordpress.net/, when run locally as VS code extension, the issue does not appear.

Block inserter "+" icon is not styled; some other UI elements (like buttons when inserting new image block, buttons on starter template on columns block, etc) are not styled as well; when inserting new image and uploading it through interface, the image does not show in block editor.

Tested on Firefox developer version 118 b9 and Chrome dev version 118 on Windows 10.

Steps to reproduce:
Start new WP instance in playground. Create new post, insert image from local file.

Related

@code-flow
Copy link

Same here (latest MacOS Firefox and Chrome). Anyone know how to fix this?

@adamziel adamziel added the [Type] Bug An existing feature does not function as intended label Sep 28, 2023
@adamziel
Copy link
Collaborator

Network inspector shows a bunch of 404s for URLs like https://playground.wordpress.net/scope:0.5858414608182674/wp-includes/css/dist/block-editor/style.min.css?ver=6.3 which originate from an iframe rendered by Gutenberg. That iframe has src="blob:https://playground.wordpress.net/9c82f60a-1f1e-4abe-adde-6b5b62f51043" and I'm almost certain it's considered cross-origin and thus the requests it issues are not handled by the service worker. It's this issue all over again:

#42

I'm not sure what the fix is yet.

@adamziel
Copy link
Collaborator

Here's what I found:

  • The iframe has the same origin as the parent window
  • The iframe still isn't being controlled by the service worker
  • The iframe can't register its own service worker

Here's the script I used to verify the above:

async function init() {
    // Register a service worker
    await navigator.serviceWorker.register('/sw.js', { scope: '/' });
    console.log('Registration successful');
    // Reload the page after the first time this script runs

    // Now, confirm the top-level script is controlled by the service worker
    console.log("I'm an top-level script", {
        origin: window.location.origin,
        swController: navigator.serviceWorker.controller
    });

    // Let's create a blob-based iframe and check if it's controlled by the service worker
    const blob = new Blob([`
    <html>
        <body>
            <h1>Test iframe</h1>
            <script>
                console.log("I'm an iframe", {
                    origin: window.location.origin,
                    swController: navigator.serviceWorker.controller,
                    protocol: window.location.protocol
                });
            </script>
        </body>
    </html>
    `], { type: 'text/html' });
    const blobUrl = URL.createObjectURL(blob);
    const iframe = document.createElement('iframe');
    document.body.appendChild(iframe);
    iframe.src = blobUrl;

    // And sadly, the iframe is not controlled by the service worker :-(
}
init();

@adamziel
Copy link
Collaborator

adamziel commented Sep 29, 2023

To sum it all up:

  • An iframe using src set to a binary blob isn't controlled by a service worker
  • An iframe using srcdoc had a null origin and also isn't controlled by a service worker
  • Ditto for using a data URL
  • An iframe loaded using a regular src="" URL worked well

URL-based src like src="/doc.html" seems to be the only way forward. Or am I missing something? cc @dmsnell

@ellatrix can you remember what was the rationale behind using a blob vs a URL? Was it saving an extra request and figuring out the right file path? Would it be viable to create an extension point to enable forcing src="file.html" in Playground?

@dmsnell
Copy link
Member

dmsnell commented Sep 29, 2023

no idea @adamziel, though I think the blob has memory implications for Gutenberg.

@ellatrix
Copy link
Member

ellatrix commented Oct 2, 2023

We switched to blob to fix the origin issue, it's also a bit cleaner that srcDoc. Why does it need to be controlled by the service worker? It's purely added in JS, nothing server side? Are we talking about the editor content iframe or something else? I thought everything was working fine?

@adamziel
Copy link
Collaborator

adamziel commented Oct 2, 2023

@ellatrix the root problem was always the service worker not handling the requests from the editor content iframe – like loading style.css for specific blocks. When it does, we say the iframe is controlled by the service worker.

In our chat I somehow reached a wrong conclusion. I thought that the iframe only needs to have the same origin to be controlled. It doesn't. The blob-based editor content iframe now has the same origin as the top-level page, but it still isn't controlled. The only way I was able to make it controlled was by using a path-based src like src="/document.html"

@adamziel adamziel mentioned this issue Oct 4, 2023
10 tasks
@adamziel
Copy link
Collaborator

adamziel commented Oct 5, 2023

We'd need to revert WordPress/gutenberg#50875 in Gutenberg, but then the original issue it solved would return:

The problem with srcDoc is that window.location is not filled, so relative hash links will trigger a navigation change and reload the page. It would also remove the need for a hack in WP playground.

For now I'll see if I can patch this in Playground to fix all the 404s, and then let's see what a proper solution would look like. cc @ellatrix

Edit: This is more difficult than just swapping src={blob} with src={url} because the initial document now loads all related CSS assets. Hm.

Edit 2: This worked for me locally:

In WordPress/Gutenberg:

src: '/wp-includes/empty.html?doc='+encodeURIComponent(renderToString( styleAssets ))

In empty.html:

<!doctype html>
<script>document.write(decodeURIComponent(window.location.search.substring(5)))</script>

However, this creates an XSS vulnerabilty. It's fine for Playground since you can run custom code anyway, but it wouldn't work for WordPress core at all. I do not see a single solution that would suit both WordPress core and Playground here and we may be stuck with patching that part of WordPress until there is one.

@adamziel adamziel changed the title CSS missing, images missing Missing CSS and images because the Gutenberg iframe isn't controlled by the service worker Oct 6, 2023
@bfintal
Copy link

bfintal commented Oct 8, 2023

Encountered this as well during the brief period the Live Preview button was in the Plugin Directory. A bunch of 404s and unstyled content of the Block Editor.

Was using latest Arc on latest MacOS.

Would recommend making this urgent issue.

@ellatrix
Copy link
Member

ellatrix commented Oct 9, 2023

URL-based src like src="/doc.html" seems to be the only way forward. Or am I missing something? cc @dmsnell

It's using a blob URL, no? 😉

@adamziel
Copy link
Collaborator

adamziel commented Oct 9, 2023

Sure it does @ellatrix. Let's call them HTTP URLs and blob URLs then :p

adamziel added a commit that referenced this issue Oct 9, 2023
…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 added a commit that referenced this issue Oct 9, 2023
…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 added a commit that referenced this issue Oct 9, 2023
…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
Copy link
Collaborator

adamziel commented Oct 9, 2023

Short-term fix coming in #668
Long-term fix is being explored in the Gutenberg repo at WordPress/gutenberg#55152

adamziel added a commit that referenced this issue Oct 9, 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
Copy link
Collaborator

adamziel commented Oct 9, 2023

#668 just landed and I'm about to deploy it to playground.wordpress.net. Let's keep this issue open until the upstream Gutenberg PR gets resolved in one way or another.

@adamziel adamziel reopened this Oct 9, 2023
adamziel added a commit that referenced this issue 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 issue 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 issue 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 issue 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.
@adamziel adamziel added this to the Zero Crashes milestone Feb 29, 2024
@adamziel adamziel moved this to Project: Up Soon in Playground Board Jun 30, 2024
@adamziel adamziel moved this from Project: Triage to Project: Up soon in Playground Board Jul 1, 2024
@adamziel adamziel moved this from Project: Up soon to Inbox in Playground Board Jul 1, 2024
@adamziel adamziel removed this from the Zero Crashes milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Aspect] Service Worker [Priority] High [Type] Bug An existing feature does not function as intended [Type] Project
Projects
Status: Inbox
6 participants