Prevent freeform and shortcode blocks from converting HTML entities #51123
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
youknowriad
added
[Type] Bug
youknowriad
commented
Jun 1, 2023
| @@ -94,9 +94,6 @@ describe( 'invalid blocks', () => { | |||
|
|
|||
| // Give the browser time to show the alert. | |||
| await page.evaluate( () => new Promise( window.requestIdleCallback ) ); | |||
|
|
|||
| expect( console ).toHaveWarned(); | |||
| expect( console ).toHaveErrored(); | |||
There was a problem hiding this comment.
The console logs were triggered because the block was considered invalid, it's now valid, I don't see why the block should be invalid here.
There was a problem hiding this comment.
I think this looks good. Expectation below remains unchanged and that's what matters.
|
Size Change: +746 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
ellatrix
approved these changes
Jun 1, 2023
|
Flaky tests detected in 48531b7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5141861433
|
|
👍 curious to see how this goes. if it fixes things that will make me quite happy. sorry I didn't respond; I was thinking about it yesterday and scheduling some time where I could more carefully consider it. |
|
@dmsnell I pinged because I was wondering if you think there's a way to replace the |
it was on my mind! I wrote a bunch of things here and then deleted them. it's possible that your change here is exactly the right thing to do, because when I looked into it's as if that is, we have an inherent conflict as we did when I introduced as I keep thinking it through in my head I think, despite expectations, the editor is doing what it should here. the one final aspect is that we could be seeing the problem with shortcodes. they should be decoding their contents so that these changes wouldn't be important (same with block validation). |
|
There's actually a number of open issues from folks complaining about the editor not allowing them to enter HTML entities and edit them (not just in these blocks, in all blocks: paragraphs, headings...). Not saying your reasoning is not valid but I know for sure that the auto-conversion was not something that we intended. |
|
all good. we may be talking about the same thing. I'm suggesting that the "html" type as we call it isn't HTML; it's parsed DOM and therefore demands conversion, because once a value is in the DOM there are no character references anymore - it's just a string whose value is what it is. there's probably all sorts of places that should be "raw" instead, which mean "the actual HTML." if the attribute is bound for |
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
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.
closes #26362
closes #33813
closes #49664
What?
One unexpected side effect of using the
html/textsource for a block attribute is that HTML entities get converted automatically when parsing the block. That's because the library we use to parse the HTMLhpqrelies onelement.innerHTMLorelement.textContentto parse the HTML and browsers automatically convert entities when doing that. This can lead to unexpected results for some blocks like the freeform/html or shortcode blocks. I think ideally our attribute parser shouldn't touch the markup at all and not perform any conversion but that's very hard to do with our current hpq approach. As a workaround for some of these blocks we can switch to therawattribute sTesting Instructions
1- Add a shortcode block
2- Paste the following shortcode
[wp_random_shortcode url="https://wordpress.org?a=x&b=y"]3- the
&shouldn't get encoded.It's very hard to predict exactly how this will impact the blocks but it seems that "not touching the source" is a better behavior.