Rich Text: also strip object replacement character when removing padding #34851
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
gwwar
added
[Type] Bug
|
Size Change: -279 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
gwwar
force-pushed
the
fix/object-replacement-character
branch
from
febf219 to
43d7c2c
Compare
mkaz
requested changes
Sep 15, 2021
mkaz
approved these changes
Sep 15, 2021
|
Probably I shouldn't get too involved here because I'm sensitive to this stuff and ignorant about what's going on. Still, why are we dealing with |
@dmsnell The TL;DR is that it's used internally in rich text. We should ignore any user provided usages to avoid errors being thrown during tree creation. |
That makes sense. This isn't really about padding then so much as it is about removing reserved characters…? I guess that's what causes the crash? You enter a |
💯 yes that's the gist of it. |
Interesting that users are pasting this character. Where are they getting this from? Maybe it's leaking somewhere when copying text? It's interesting that this only starts happening now, we've been using this character internally for a long time. |
ellatrix
reviewed
Sep 16, 2021
getdave
reviewed
Sep 16, 2021
| expect( removeReservedCharacters( `${ ZWNBSP }` ) ).toEqual( '' ); | ||
| expect( | ||
| removeReservedCharacters( | ||
| `${ OBJECT_REPLACEMENT_CHARACTER }c${ OBJECT_REPLACEMENT_CHARACTER }at${ OBJECT_REPLACEMENT_CHARACTER }` |
ellatrix
approved these changes
Sep 16, 2021
getdave
approved these changes
Sep 16, 2021
|
Thanks for the reviews @ellatrix @getdave @dmsnell @mkaz ! I'll go ahead and merge this once tests are green.
I asked folks and some reports are from users who are copying text from MS Word. It's definitely something to keep an eye on though! |
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.
Fixes #34602. Rich text instances should ignore the object replacement character U+FFFC, since this is used internally by the package.
Changes here strip this special character when we remove other whitespace characters like
u+feff,Testing Instructions
This post title crashes Gutenberg Verify that no JS errors are thrown