Block Supports: Add missing UTF-8 conversion #24447

ockham · 2020-08-07T14:56:07Z

Description

Fixes #24445.

This should probably be backported to 8.7 (and potentially further back if needed).

How has this been tested?

See #24445 for instructions on how to reproduce the issue. Verify that this PR fixes it.

Types of changes

Bug fix.

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.

props @akirk @sirreal @simison for finding the fix

mcsf · 2020-08-07T14:58:59Z

@swissspidy: I'd love your eyes on this, if you can spare some time ~~today~~ (edit: not urgent as initially thought).

ockham · 2020-08-07T15:02:04Z

The fix for 8.7 needs to be applied to lib/blocks.php. Unit tests probably will continue work without change.

(Code was moved and files renamed by #24351)

github-actions · 2020-08-07T15:02:45Z

Size Change: +381 B (0%)

Total Size: 1.16 MB

Filename	Size	Change
`build/block-editor/index.js`	125 kB	+60 B (0%)
`build/block-editor/style-rtl.css`	10.6 kB	+34 B (0%)
`build/block-editor/style.css`	10.6 kB	+36 B (0%)
`build/block-library/index.js`	132 kB	-172 B (0%)
`build/block-library/style-rtl.css`	7.76 kB	+1 B
`build/block-library/style.css`	7.77 kB	+1 B
`build/blocks/index.js`	48.4 kB	+118 B (0%)
`build/components/index.js`	200 kB	-5 B (0%)
`build/components/style-rtl.css`	15.7 kB	+2 B (0%)
`build/components/style.css`	15.7 kB	+2 B (0%)
`build/edit-post/index.js`	304 kB	+273 B (0%)
`build/edit-post/style-rtl.css`	5.62 kB	+22 B (0%)
`build/edit-post/style.css`	5.61 kB	+21 B (0%)
`build/editor/index.js`	45.3 kB	-13 B (0%)
`build/element/index.js`	4.65 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.44 kB	0 B
`build/autop/index.js`	2.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	7.97 kB	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`	7.59 kB	0 B
`build/block-library/editor.css`	7.59 kB	0 B
`build/block-library/theme-rtl.css`	729 B	0 B
`build/block-library/theme.css`	730 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/compose/index.js`	9.68 kB	0 B
`build/core-data/index.js`	11.8 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.45 kB	0 B
`build/date/index.js`	5.38 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.23 kB	0 B
`build/edit-navigation/index.js`	10.9 kB	0 B
`build/edit-navigation/style-rtl.css`	1.08 kB	0 B
`build/edit-navigation/style.css`	1.08 kB	0 B
`build/edit-site/index.js`	17 kB	0 B
`build/edit-site/style-rtl.css`	3.06 kB	0 B
`build/edit-site/style.css`	3.06 kB	0 B
`build/edit-widgets/index.js`	9.38 kB	0 B
`build/edit-widgets/style-rtl.css`	2.45 kB	0 B
`build/edit-widgets/style.css`	2.45 kB	0 B
`build/editor/editor-styles-rtl.css`	537 B	0 B
`build/editor/editor-styles.css`	539 B	0 B
`build/editor/style-rtl.css`	3.8 kB	0 B
`build/editor/style.css`	3.79 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.72 kB	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`	711 B	0 B
`build/keyboard-shortcuts/index.js`	2.52 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.11 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.33 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 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/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	13.9 kB	0 B
`build/server-side-render/index.js`	2.71 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/url/index.js`	4.06 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}

mcsf · 2020-08-07T15:14:14Z

OK, so this is not affecting WP 5.5 (feel free to double-check), which makes sense considering that the functionality in question (Global Styles-related block support) was intentionally merged after code freeze.

I'll update the metadata accordingly.

sirreal · 2020-08-07T15:14:44Z

I've also tested and confirmed that this does not appear to affect WordPRess 5.5.

sirreal

I am not an expert in encodings nor DOM manipulations via PHP.

I have confirmed this bug is present in Gutenberg 8.7.

I have tested and confirms that this fixes it in my testing:

I've created a post with a Cover block containing the text こんにちは世界.

Without the patch:

With the patch applied (the block is un-edited):

This may be worth an 8.7.1 release.

ockham · 2020-08-07T16:11:34Z

FAIL packages/e2e-tests/specs/editor/various/adding-blocks.test.js (26.685s)
  ● adding blocks › inserts a block in proper place after having clicked `Browse All` from inline inserter

    expect(received).toMatchSnapshot()

    Snapshot name: `adding blocks inserts a block in proper place after having clicked \`Browse All\` from inline inserter 2`

    - Snapshot  - 4
    + Received  + 0

    @@ -1,13 +1,9 @@
      <!-- wp:paragraph -->
      <p>First paragraph</p>
      <!-- /wp:paragraph -->

    - <!-- wp:cover -->
    - <div class="wp-block-cover has-background-dim"><div class="wp-block-cover__inner-container"></div></div>
    - <!-- /wp:cover -->
    -
      <!-- wp:heading -->
      <h2>Heading</h2>
      <!-- /wp:heading -->

      <!-- wp:paragraph -->

      259 | 		);
      260 | 		await coverBlock.click();
    > 261 | 		expect( await getEditedPostContent() ).toMatchSnapshot();
          | 		                                       ^
      262 | 	} );
      263 | } );
      264 | 

      at Object.<anonymous> (specs/editor/various/adding-blocks.test.js:261:42)
          at runMicrotasks (<anonymous>)

 › 1 snapshot failed.

😕

Seems like the DOM operation might remove the empty Cover div? 🤔

ockham · 2020-08-07T16:21:15Z

Seems to pass locally. I'll re-run the test.

ockham · 2020-08-07T17:16:08Z

Passing now 😌

We should add some test coverage for this though 🤔

ockham · 2020-08-07T17:19:44Z

We should add some test coverage for this though 🤔

A simple unit test for gutenberg_apply_block_supports that feeds it e.g. a wide-aligned cover block with some non-latin chars could do the trick.

ockham · 2020-08-10T12:44:37Z

I'll try writing a quick unit test for this.

ockham · 2020-08-10T14:43:06Z

Added a unit test. I also discovered that I needed to add a line to convert HTML entities back to UTF-8 (after $dom->saveHtml()) in order to make sure we feed UTF-8 in, we get UTF-8 (not entities) out.

ockham · 2020-08-10T14:46:06Z

FWIW, cherry-picked the unit test commit on top of master, ran them: They fail the block content check that I added, just like they should 🎉

* Block Supports: Add missing UTF-8 conversion * Re-encode back to UTF-8 * Add unit test

Sets the charset of the html passed into DomDocument to utf-8. Replaces the mb_convert_encoding call replacing utf-8 with html entities before handing off to DomDocument. This avoids the need to later convert back to utf-8 from html entities afterward. This secondary mb_convert_encoding call was converting not only the utf-8 we converted earlier but also entity encoding html stored inside data-* or other attributes of html elements. Fixes Automattic/wp-calypso#44897 Maintains the fix for WordPress#24445 (WordPress#24447)

Block Supports: Add missing UTF-8 conversion

01bd02f

ockham added [Type] Bug An existing feature does not function as intended [Priority] High Used to indicate top priority items that need quick attention labels Aug 7, 2020

ockham requested review from mcsf, youknowriad, gziolo and epiqueras August 7, 2020 14:56

ockham requested review from spacedmonkey and TimothyBJacobs as code owners August 7, 2020 14:56

ockham self-assigned this Aug 7, 2020

sirreal added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 7, 2020

mcsf removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 7, 2020

sirreal approved these changes Aug 7, 2020

View reviewed changes

sirreal mentioned this pull request Aug 7, 2020

Wide-aligned blocks: Non latin text characters are rendered as gibberish inside the block if set to Wide or Full Width #24445

Closed

mcsf mentioned this pull request Aug 10, 2020

Character encoding issue on front end - (â€™) instead of apostraphe #24384

Closed

mcsf mentioned this pull request Aug 10, 2020

Site Title: Add support settings for colors, fonts, and line height #23007

Merged

12 tasks

ockham added 2 commits August 10, 2020 16:39

Re-encode back to UTF-8

5ea13cd

Add unit test

051d893

ockham merged commit 8474a2e into master Aug 11, 2020

ockham deleted the fix/align-wide-breaks-unicode branch August 11, 2020 08:47

github-actions bot added this to the Gutenberg 8.8 milestone Aug 11, 2020

youknowriad pushed a commit that referenced this pull request Aug 11, 2020

Block Supports: Add missing UTF-8 conversion (#24447)

55d228f

* Block Supports: Add missing UTF-8 conversion * Re-encode back to UTF-8 * Add unit test

aktasfatih mentioned this pull request Aug 11, 2020

Having problems with utf-8 #24477

Closed

ockham mentioned this pull request Aug 17, 2020

Images with descriptions output HTML as well as image Automattic/wp-calypso#44897

Closed

ItsJonQ mentioned this pull request Aug 17, 2020

Bump plugin version to 8.8.0 #24601

Closed

lsl mentioned this pull request Aug 19, 2020

Avoid lossy html entities encoding by setting charset #24645

Merged

6 tasks

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: Add missing UTF-8 conversion #24447

Block Supports: Add missing UTF-8 conversion #24447

ockham commented Aug 7, 2020 •

edited

Loading

mcsf commented Aug 7, 2020 •

edited

Loading

ockham commented Aug 7, 2020 •

edited

Loading

github-actions bot commented Aug 7, 2020 •

edited

Loading

mcsf commented Aug 7, 2020

sirreal commented Aug 7, 2020

sirreal left a comment •

edited

Loading

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 10, 2020

ockham commented Aug 10, 2020

ockham commented Aug 10, 2020

Block Supports: Add missing UTF-8 conversion #24447

Block Supports: Add missing UTF-8 conversion #24447

Conversation

ockham commented Aug 7, 2020 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

mcsf commented Aug 7, 2020 • edited Loading

ockham commented Aug 7, 2020 • edited Loading

github-actions bot commented Aug 7, 2020 • edited Loading

mcsf commented Aug 7, 2020

sirreal commented Aug 7, 2020

sirreal left a comment • edited Loading

Choose a reason for hiding this comment

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 7, 2020

ockham commented Aug 10, 2020

ockham commented Aug 10, 2020

ockham commented Aug 10, 2020

ockham commented Aug 7, 2020 •

edited

Loading

mcsf commented Aug 7, 2020 •

edited

Loading

ockham commented Aug 7, 2020 •

edited

Loading

github-actions bot commented Aug 7, 2020 •

edited

Loading

sirreal left a comment •

edited

Loading