Block Supports: Ensure consistent output in different PHP versions #25240

sirreal · 2020-09-11T11:34:59Z

Description

Avoid using the DOMDocument::saveHtml( $node ) to ensure consistent behavior across PHP versions.

In some versions of PHP (< 7.3) the DOMDocument::saveHtml( $node ) method would format HTML introducing whitespace that could result in different rendered results in the browser.

For example, the following snippet will demonstrate different output in HTML whitespace when using saveHtml with the $node argument compared with saving the entire document:

<?php

$dom = new DOMDocument();
$dom->loadHTML( "<!DOCTYPE html><html><body><ul><li>first</li><li>second</li><li>third</li></ul></html>" );
$ul = $dom->getElementsByTagName('ul')->item(0);

echo "Element node:" . PHP_EOL;
echo $dom->saveHTML( $ul ) . PHP_EOL;

echo "Root + substr:" . PHP_EOL;
$full_html = $dom->saveHtml();
$start = mb_strpos( $full_html, '<body>', 0, 'UTF-8' ) + mb_strlen( '<body>', 'UTF-8' );
$end   = mb_strpos( $full_html, '</body>', $start, 'UTF-8' );
echo mb_substr( $full_html, $start, $end - $start, 'UTF-8' );

Results PHP 7.3+

Element node:
<ul><li>first</li><li>second</li><li>third</li></ul>
Root + substr:
<ul><li>first</li><li>second</li><li>third</li></ul>

Results PHP 5.3.6 - 7.2: (earlier versions do not support the $node argument to saveHtml)

Element node:
<ul>
<li>first</li>
<li>second</li>
<li>third</li>
</ul>
Root + substr:
<ul><li>first</li><li>second</li><li>third</li></ul>

Addresses concerns raised here:
#25028 (comment)

How has this been tested?

Unit tests.

Types of changes

Internal: Fixes a potential bug specific to certain PHP versions.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

In some versions of PHP (< 7.3) the `DOMDocument::saveHtml( $node )` method would format HTML introducing whitespace that could result in different rendered results in the browser. Avoid using the `DOMDocument::saveHtml( $node )` to ensure consistent behavior with supported PHP versions. Mentioned here: #25028 (comment)

github-actions · 2020-09-11T11:43:30Z

Size Change: +1.58 kB (0%)

Total Size: 1.2 MB

Filename	Size	Change
`build/autop/index.js`	2.83 kB	+1 B
`build/block-directory/index.js`	8.53 kB	-17 B (0%)
`build/block-editor/index.js`	128 kB	-22 B (0%)
`build/block-editor/style-rtl.css`	11.1 kB	-4 B (0%)
`build/block-editor/style.css`	11.1 kB	-4 B (0%)
`build/block-library/index.js`	139 kB	+67 B (0%)
`build/blocks/index.js`	47.8 kB	+2 B (0%)
`build/components/index.js`	202 kB	+1.39 kB (0%)
`build/compose/index.js`	9.67 kB	-5 B (0%)
`build/core-data/index.js`	12.4 kB	+7 B (0%)
`build/data/index.js`	8.55 kB	+9 B (0%)
`build/date/index.js`	31.9 kB	+1 B
`build/edit-navigation/index.js`	10.7 kB	-1 B
`build/edit-post/index.js`	305 kB	+3 B (0%)
`build/edit-post/style-rtl.css`	6.25 kB	-11 B (0%)
`build/edit-post/style.css`	6.23 kB	-11 B (0%)
`build/edit-site/index.js`	19.3 kB	-1 B
`build/edit-site/style-rtl.css`	3.14 kB	+78 B (2%)
`build/edit-site/style.css`	3.14 kB	+77 B (2%)
`build/edit-widgets/index.js`	12.2 kB	+19 B (0%)
`build/editor/index.js`	45.6 kB	+3 B (0%)
`build/element/index.js`	4.65 kB	-1 B
`build/format-library/index.js`	7.71 kB	-2 B (0%)
`build/keyboard-shortcuts/index.js`	2.52 kB	-1 B
`build/list-reusable-blocks/index.js`	3.12 kB	-1 B
`build/nux/index.js`	3.4 kB	+5 B (0%)
`build/redux-routine/index.js`	2.85 kB	+3 B (0%)
`build/rich-text/index.js`	13.9 kB	+3 B (0%)
`build/url/index.js`	4.06 kB	-1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.67 kB	0 B
`build/api-fetch/index.js`	3.41 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	953 B	0 B
`build/block-directory/style.css`	952 B	0 B
`build/block-library/editor-rtl.css`	8.68 kB	0 B
`build/block-library/editor.css`	8.68 kB	0 B
`build/block-library/style-rtl.css`	7.6 kB	0 B
`build/block-library/style.css`	7.59 kB	0 B
`build/block-library/theme-rtl.css`	754 B	0 B
`build/block-library/theme.css`	754 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/components/style-rtl.css`	15.5 kB	0 B
`build/components/style.css`	15.5 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	4.47 kB	0 B
`build/edit-navigation/style-rtl.css`	868 B	0 B
`build/edit-navigation/style.css`	871 B	0 B
`build/edit-widgets/style-rtl.css`	2.55 kB	0 B
`build/edit-widgets/style.css`	2.55 kB	0 B
`build/editor/editor-styles-rtl.css`	492 B	0 B
`build/editor/editor-styles.css`	493 B	0 B
`build/editor/style-rtl.css`	3.81 kB	0 B
`build/editor/style.css`	3.81 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	621 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/media-utils/index.js`	5.32 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/style-rtl.css`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.41 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/server-side-render/index.js`	2.77 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/viewport/index.js`	1.85 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

lib/block-supports/index.php

westonruter

LGTM

sirreal marked this pull request as ready for review September 11, 2020 11:37

sirreal requested review from spacedmonkey and TimothyBJacobs as code owners September 11, 2020 11:37

sirreal self-assigned this Sep 11, 2020

sirreal added [Feature] Blocks Overall functionality of blocks Needs Technical Feedback Needs testing from a developer perspective. labels Sep 11, 2020

sirreal requested a review from westonruter September 11, 2020 11:37

sirreal mentioned this pull request Sep 11, 2020

Block supports: Restore root DOMDocument save #25028

Merged

sirreal requested a review from ockham September 11, 2020 11:38

westonruter requested changes Sep 11, 2020

View reviewed changes

lib/block-supports/index.php Outdated Show resolved Hide resolved

sirreal added 2 commits September 11, 2020 21:23

Drop mb_ versions

20646ab

Trim result

d104c75

sirreal requested a review from westonruter September 11, 2020 19:24

westonruter approved these changes Sep 11, 2020

View reviewed changes

sirreal merged commit 6671cef into master Sep 11, 2020

sirreal deleted the update/ensure-savehtml-compatibility branch September 11, 2020 21:14

github-actions bot added this to the Gutenberg 9.0 milestone Sep 11, 2020

mcsf mentioned this pull request Oct 14, 2020

Try: Reimplement Block Supports #26111

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Supports: Ensure consistent output in different PHP versions #25240

Block Supports: Ensure consistent output in different PHP versions #25240

sirreal commented Sep 11, 2020 •

edited

github-actions bot commented Sep 11, 2020 •

edited

westonruter left a comment

Block Supports: Ensure consistent output in different PHP versions #25240

Block Supports: Ensure consistent output in different PHP versions #25240

Conversation

sirreal commented Sep 11, 2020 • edited

Description

How has this been tested?

Types of changes

Checklist:

github-actions bot commented Sep 11, 2020 • edited

westonruter left a comment

Choose a reason for hiding this comment

sirreal commented Sep 11, 2020 •

edited

github-actions bot commented Sep 11, 2020 •

edited