Skip to content

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

Merged
sirreal merged 3 commits intomasterfrom
update/ensure-savehtml-compatibility
Sep 11, 2020
Merged

Block Supports: Ensure consistent output in different PHP versions#25240
sirreal merged 3 commits intomasterfrom
update/ensure-savehtml-compatibility

Conversation

@sirreal
Copy link
Copy Markdown
Member

@sirreal sirreal commented Sep 11, 2020

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)
@sirreal sirreal marked this pull request as ready for review September 11, 2020 11:37
@sirreal sirreal self-assigned this Sep 11, 2020
@sirreal sirreal added [Feature] Blocks Overall functionality of blocks Needs Technical Feedback Needs testing from a developer perspective. labels Sep 11, 2020
@sirreal sirreal requested a review from westonruter September 11, 2020 11:37
@sirreal sirreal requested a review from ockham September 11, 2020 11:38
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 11, 2020

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

@sirreal sirreal requested a review from westonruter September 11, 2020 19:24
Copy link
Copy Markdown
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sirreal sirreal merged commit 6671cef into master Sep 11, 2020
@sirreal sirreal deleted the update/ensure-savehtml-compatibility branch September 11, 2020 21:14
@github-actions github-actions bot added this to the Gutenberg 9.0 milestone Sep 11, 2020
@mcsf mcsf mentioned this pull request Oct 14, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Blocks Overall functionality of blocks Needs Technical Feedback Needs testing from a developer perspective.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants