Save DOMDocument node instead of root + str_replace #25020
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
We wrapped block HTML in some tags to ensure the DOMDocument parses it correctly, then unwrapped it again with str_replace after using `saveHTML`. The `saveHTML` method accepts an optional argument that is the dom node to output a subset of the document: https://www.php.net/manual/en/domdocument.savehtml.php Rather than calling `str_replace` on the entire document to get the unwrapped block HTML, invoke `saveHTML` on the block root to get the unwrapped HTML. This prevents some additional newlines from being appended to the output which was observed when running core block tests with Gutenberg activated.
sirreal
added
the
Needs Technical Feedback
sirreal
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: +30 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
|
Looking good. @sirreal Is there any chance we can modify the string we use in unit tests to cover the problematic newlines case? gutenberg/phpunit/class-block-supported-styles-test.php Lines 120 to 126 in 668182c |
ockham
approved these changes
Sep 2, 2020
sirreal
removed
the
Needs Technical Feedback
ockham
approved these changes
Sep 2, 2020
|
This PR caused a regression in the Categories block when a dropdown is displayed. See #25026. |
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.
Description
We wrapped block HTML in some tags to ensure the DOMDocument parses it
correctly, then unwrapped it again with str_replace after using
saveHTML.The
saveHTMLmethod accepts an optional argument that is the dom nodeto output a subset of the document:
https://www.php.net/manual/en/domdocument.savehtml.php
Rather than calling
str_replaceon the entire document to get theunwrapped block HTML, invoke
saveHTMLon the block root to get theunwrapped HTML.
This prevents some additional newlines from being appended to the
output which was observed when running core block tests with Gutenberg
activated.
How has this been tested?
Manual and automated testing of some dynamic blocks.
Types of changes
Internal.
Checklist: