New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize unicode in raw handling #6880

Merged
merged 1 commit into from May 22, 2018

Conversation

Projects
None yet
4 participants
@iseulde
Member

iseulde commented May 21, 2018

Description

Fixes #433 and https://core.trac.wordpress.org/ticket/30130 for core.

This branch fixes an issue when copy pasting decomposed characters from an external source. The characters should be composed (normalised) so matching works properly. Decomposed characters can also appear strange in some browsers.

How has this been tested?

See https://core.trac.wordpress.org/ticket/30130 for a PDF to test. Paste the content in Gutenberg in e.g. Safari where the appearance is strange. Also try searching one of the pasted words but in composed form, which will fail.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@iseulde iseulde requested a review from WordPress/gutenberg-core May 21, 2018

@noisysocks

Tested this locally in Safari. It fixes the problem 👍

@iseulde

This comment has been minimized.

Member

iseulde commented May 22, 2018

Thanks for the quick review!

@iseulde iseulde merged commit cd5779d into master May 22, 2018

2 checks passed

codecov/project 46.45% (+<.01%) compared to 5a91c0a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@iseulde iseulde deleted the add/raw-handling-normalize branch May 22, 2018

@danielbachhuber danielbachhuber added this to the 3.0 milestone May 22, 2018

@@ -88,6 +88,11 @@ export default function rawHandler( { HTML = '', plainText = '', mode = 'AUTO',
return parseWithGrammar( HTML );
}
// Normalize unicode to use composed characters.
// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize

This comment has been minimized.

@aduth

aduth May 23, 2018

Member

From this link, it is stated there is no support for Internet Explorer:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize#Browser_compatibility

Maybe we want a polyfill?

https://github.com/walling/unorm

Prior art for environment-specific polyfills:

gutenberg_get_script_polyfill( array(
'\'Promise\' in window' => 'promise',
'\'fetch\' in window' => 'fetch',
) ),

This comment has been minimized.

@iseulde

iseulde May 23, 2018

Member

Hm, thanks for catching that. In this case I'm more tempted to drop the normalisation. What do you think?

This comment has been minimized.

@aduth

aduth May 25, 2018

Member

As a short-term fix for the next release?

Otherwise, how much of an issue is the lack of normalization?

It's not too problematic to add a polyfill if it's well-implemented. It will only be loaded for unsupporting browsers, and doesn't have much of an impact otherwise.

@aduth

This comment has been minimized.

Member

aduth commented May 25, 2018

Opened #6958 for tracking issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment