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

Rendering legacy widgets preview via an iframe may lead to a 414 URI too long error #33540

Open
adamziel opened this issue Jul 19, 2021 · 10 comments
Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@adamziel
Copy link
Contributor

adamziel commented Jul 19, 2021

Description

Legacy widgets are rendered using an <iframe /> with src set like this:

src={ addQueryArgs( 'widgets.php', {
'legacy-widget-preview': {
idBase,
instance,
},
} ) }

This works when there aren't many parameters or when the server tolerates very long query strings. Unfortunately, when one of these conditions isn't met, we may see an error 414 URI too long like on the screenshot below:

Zrzut ekranu 2021-07-19 o 12 32 37

This is easiest to reproduce with a long piece of text, but it also occurs with short inputs when certain plugins are installed. For example, the https://pl.wordpress.org/plugins/widget-visibility-time-scheduler/ adds a number of form fields to every legacy widget's form and triggers errors 414 in certain configurations.

Step-by-step reproduction instructions

  1. Install and activate the "Classic widgets" plugin
  2. Add a new text widget
  3. Paste a few paragraphs of lorem ipsum and save
  4. Deactivate the classic widgets plugin
  5. Go to widgets.php

Expected behaviour

A correctly rendered preview

Actual behaviour

Error 414 (if there isn't one, just add a few more paragraphs of text and try again)

@adamziel adamziel added [Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Package] Edit Widgets /packages/edit-widgets [Feature] Widgets Customizer Ability to add and edit blocks in Customize → Widgets. labels Jul 19, 2021
@adamziel
Copy link
Contributor Author

One way to solve it would involve switching to POST requests and "manually" populating the preview iframe.

@adamziel
Copy link
Contributor Author

I started exploring a solution in WordPress/wordpress-develop#1508

@noisysocks
Copy link
Member

This ties into "Investigate alternatives to using an iframe in Legacy Widget block" on #33242.

I was wondering if we could look at using ServerSideRender instead of an iframe to show the preview. The only issue is that we'd need to separate the markup that comes back from the server from the editor markup. We can maybe do this using shadow DOM or by putting the ServerSideRender in a portal which renders in an iframe.

@noisysocks noisysocks added this to the WordPress 5.8.1 milestone Jul 20, 2021
@adamziel
Copy link
Contributor Author

adamziel commented Jul 20, 2021

@noisysocks I actually had a working prototype of ServerSideRender but then I realized we need to render a full HTML document with wp_head(), CSS links, maybe some additional scripts, and this is much more that I would like to return from a block rendering API. It seems to fit nicely into our widgets-related API though. What do you think? Either way, we need to update some endpoint to make it produce that HTML.

@adamziel
Copy link
Contributor Author

adamziel commented Jul 27, 2021

The API request PR is not a draft anymore: WordPress/wordpress-develop#1508

@noisysocks would you mind reviewing?

@adamziel
Copy link
Contributor Author

adamziel commented Aug 16, 2021

ping @noisysocks @kevin940726 @draganescu

@kevin940726
Copy link
Member

I might be missing some context here, but switching to POST request alone doesn't seem to be enough? We're still depending on iframes to render the previews in isolation, and AFAIK, iframe can't load a POST request url. We'll need something else to use JavaScript to render the response from that POST request. Is there any plan about that? @noisysocks's portal and shadow DOM approaches seem promising. Might want to prove that they work before merging the POST api PR.

@adamziel
Copy link
Contributor Author

adamziel commented Aug 17, 2021

@kevin940726 you can perform the POST request via fetch and then use the response to populate a new, empty iframe via Javascript. I don't think shadow DOM would handle scripts well.

@kevin940726
Copy link
Member

Oh I see, we can use srcdoc for that, I totally forgot about that! The only difference might only be the lack of url, but I think it should be fine in our case.

@noisysocks
Copy link
Member

While we're fiddling around in here, let's add better error handling to this iframe. Many times an HTTP error shows up in the iframe with an error message that is not actionable. This makes it pretty hard to offer support to users who encounter errors. For example, look at #32648. We should try to present the message field if we got a REST API error payload, or, failing that, present the HTTP error code. In all cases we should present a Retry button.

@noisysocks noisysocks added [Type] Bug An existing feature does not function as intended Needs Dev Ready for, and needs developer efforts and removed [Package] Edit Widgets /packages/edit-widgets [Feature] Widgets Customizer Ability to add and edit blocks in Customize → Widgets. labels Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants