Fix unwanted additional spaces added around pasted text on Windows #33607

getdave · 2021-07-21T18:34:31Z

Description

When pasting text internally within the editor on Windows additional spaces are added to either side of the pasted text.

Copied text (i.e. what the user intended to copy) two.
Resulting text when pasted two.

This appears to be due to windows providing additional unwanted data when retrieving text/html from the clipboard event data. The actual content copied is:

<html>
 <body>
   <!--StartFragment-->
   Copied text here
   <!--EndFragment-->
 </body>
</html>

When this passes through the editor's paste mechanics and the format API we end up with spaces around the text.

To fix this, this PR attempts to use regex to remove:

The two "Fragment" comments.
Whitespace either side of the comments.
It does not remove whitespace inside the "Fragment" comments because the user may have intentionally copied some spaces which we would want to preserve.

Fixes #33283

How has this been tested?

Use Windows 10 and latest MS Edge browser.
Create a new Post.
Write some text such as Hello I am some text. Let's copy me.
Using your mouse highlight a word in that text being sure not to copy any whitespace.
Paste the copied text into the middle of another word.
See that there are no spaces added either side of the text your pasted.
Repeat but this time copy some text that does include some whitespace either side. We need to check that in this scenario the whitespace is preserved.

Also test on Mac to ensure it doesn't regress this implementation.

Screenshots

Windows

Before

Screen.Capture.on.2021-07-21.at.17-05-19.mov

After

Screen.Capture.on.2021-07-22.at.11-59-51.mp4

Mac OS

The below shows only the after fix as the bug did not manifest itself on Mac even before this fix.

Screen.Capture.on.2021-07-22.at.11-58-39.mp4

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
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 (please manually search all *.native.js files for terms that need renaming or removal).

See https://discuss.prosemirror.net/t/space-added-on-paste/1274/7

getdave · 2021-07-23T18:06:36Z

Ok @ellatrix it looks like this solves the reported Issue. If you have time to review my implementation that would be great.

draganescu

Given that this is based on a documented OS specific issue that other editos encountered I say it's ✅ But we have a paste-handler that could handle this better than directly in Rich Text?

getdave · 2021-07-29T15:53:09Z

...we have a paste-handler that could handle this better than directly in Rich Text?

🤔 Yeh actually it might be better moved to there. Let me see...

getdave · 2021-07-29T16:05:41Z

Ok it looks like we go down the isInternal code path when pasting within the editor. That doesn't invoke the paste handler which is why we need the additional fix in this PR.

gutenberg/packages/block-editor/src/components/rich-text/use-paste-handler.js

Lines 109 to 120 in 339a7d7

    
           if ( isInternal ) { 
        
           	const pastedValue = create( { 
        
           		html, 
        
           		multilineTag, 
        
           		multilineWrapperTags: 
        
           			multilineTag === 'li' ? [ 'ul', 'ol' ] : undefined, 
        
           		preserveWhiteSpace, 
        
           	} ); 
        
           	addActiveFormats( pastedValue, value.activeFormats ); 
        
           	onChange( insert( value, pastedValue ) ); 
        
           	return; 
        
           }

@ellatrix I'd like to understand whether you agree that it's a good idea to strip the Windows specific fragment markers in all cases as I've done in this PR. If we don't do it here then it's never handled because whilst the pasteHandler does deal with the fragments in this "internal" paste scenario it is never invoked. As a result Windows exhibits this bug.

…larity As we're not sure about this fix let's not create a public API via utils. Instead revert to inlining. Also renaming the function to make it's purpose clearer. It's not a generic normalizing function so let's make it obvious this is Windows specific.

draganescu

I think this is a good fix and despite my doubts about the place where the fix is applied we can always refactor this later.

kevin940726

I tested it in Edge. Sometimes when you copy more than one words (with multiple spaces in between) and paste it somewhere, the copied texts are messed up, in a unpredictable way. Are you able to reproduce it?

getdave · 2021-08-10T12:49:30Z

I tested it in Edge. Sometimes when you copy more than one words (with multiple spaces in between) and paste it somewhere, the copied texts are messed up, in a unpredictable way. Are you able to reproduce it?

No I'm not.

Screen.Capture.on.2021-08-10.at.13-48-34.mov

Are you able to provide exact steps to reproduce?

getdave · 2021-08-10T14:51:37Z

@kevin940726 Let me know if can no longer replicate this. If you can't then I think we should look to merge the fix as it's been around for a while now and folk are waiting on the PR to land.

kevin940726

I can't reproduce the bug anymore, let's ship it 👍

packages/block-editor/src/components/rich-text/use-paste-handler.js

ellatrix · 2021-08-14T12:01:42Z

Could you please add an integration test?

kevin940726 · 2021-08-14T14:25:02Z

I believe the bug is only reproducible on Windows though, so not sure if we have the established testing environment yet.

ellatrix · 2021-08-16T16:42:39Z

@kevin940726 A lot of the integration tests we have are for Windows. We just need a copy of the HTML that was inserted. See https://github.com/WordPress/gutenberg/tree/trunk/test/integration/fixtures/documents.

Please add an integration test because issues like this are easy to regress with refactors.

ellatrix · 2021-08-16T16:43:39Z

See also https://github.com/WordPress/gutenberg/blob/trunk/test/integration/blocks-raw-handling.test.js

kevin940726 · 2021-08-17T06:28:48Z

@ellatrix Cool! Didn't know that! So it's more like a unit/integration test. For some reasons I was thinking e2e tests 😅. @getdave Care to create a follow-up PR? Or maybe someone else can do that.

getdave · 2021-08-23T13:07:57Z

Greetings 👋

Yes good point. I was thinking along same lines as Kai with the Windows specific nature of the issue, but Ella's information puts a different spin on it.

I'll put it on my list to add a test.

Remove the whitespace before the fragments that are provided by windows

245eecd

See https://discuss.prosemirror.net/t/space-added-on-paste/1274/7

getdave self-assigned this Jul 21, 2021

getdave added the [Feature] Paste label Jul 21, 2021

getdave mentioned this pull request Jul 21, 2021

Regression in Gutenberg 11 around pasting with spaces? #33283

Closed

getdave changed the title ~~Fix additional spaces added around pasted text on Windows~~ Fix unwanted additional spaces added around pasted text on Windows Jul 21, 2021

This comment has been minimized.

Sign in to view

Extract to util

d4657d8

getdave requested a review from ellatrix July 22, 2021 11:04

getdave added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Bug An existing feature does not function as intended labels Jul 22, 2021

getdave marked this pull request as ready for review July 22, 2021 11:05

getdave requested review from cameronvoell and guarani as code owners July 22, 2021 11:05

draganescu reviewed Jul 29, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

draganescu approved these changes Aug 4, 2021

View reviewed changes

kevin940726 reviewed Aug 4, 2021

View reviewed changes

getdave requested a review from kevin940726 August 10, 2021 12:50

getdave added the [Type] Regression Related to a regression in the latest release label Aug 10, 2021

kevin940726 approved these changes Aug 12, 2021

View reviewed changes

packages/block-editor/src/components/rich-text/use-paste-handler.js Outdated Show resolved Hide resolved

Make both regexes literal

6a0db13

getdave force-pushed the fix/additional-spaces-on-paste-on-windows branch from b4a86eb to 6a0db13 Compare August 12, 2021 11:12

getdave merged commit db73369 into trunk Aug 12, 2021

getdave deleted the fix/additional-spaces-on-paste-on-windows branch August 12, 2021 11:52

github-actions bot added this to the Gutenberg 11.4 milestone Aug 12, 2021

getdave mentioned this pull request Aug 23, 2021

Add integration test to cover handling of spaces on paste in Windows #34235

Open

getdave mentioned this pull request Oct 6, 2021

Pasting text adds leading and trailing spaces #16172

Closed

t-hamano mentioned this pull request Nov 12, 2022

Raw Handling: Paste from MS Excel no longer creates a table block #42570

Open

ellatrix mentioned this pull request Oct 10, 2023

Fix pasting MS Word table #55206

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unwanted additional spaces added around pasted text on Windows #33607

Fix unwanted additional spaces added around pasted text on Windows #33607

getdave commented Jul 21, 2021 •

edited

Loading

This comment has been minimized.

getdave commented Jul 23, 2021

draganescu left a comment

getdave commented Jul 29, 2021

This comment has been minimized.

getdave commented Jul 29, 2021 •

edited

Loading

draganescu left a comment

kevin940726 left a comment

getdave commented Aug 10, 2021

getdave commented Aug 10, 2021

kevin940726 left a comment

ellatrix commented Aug 14, 2021

kevin940726 commented Aug 14, 2021

ellatrix commented Aug 16, 2021

ellatrix commented Aug 16, 2021

kevin940726 commented Aug 17, 2021

getdave commented Aug 23, 2021

Fix unwanted additional spaces added around pasted text on Windows #33607

Fix unwanted additional spaces added around pasted text on Windows #33607

Conversation

getdave commented Jul 21, 2021 • edited Loading

Description

How has this been tested?

Screenshots

Windows

Before

After

Mac OS

Types of changes

Checklist:

This comment has been minimized.

getdave commented Jul 23, 2021

draganescu left a comment

Choose a reason for hiding this comment

getdave commented Jul 29, 2021

This comment has been minimized.

getdave commented Jul 29, 2021 • edited Loading

draganescu left a comment

Choose a reason for hiding this comment

kevin940726 left a comment

Choose a reason for hiding this comment

getdave commented Aug 10, 2021

getdave commented Aug 10, 2021

kevin940726 left a comment

Choose a reason for hiding this comment

ellatrix commented Aug 14, 2021

kevin940726 commented Aug 14, 2021

ellatrix commented Aug 16, 2021

ellatrix commented Aug 16, 2021

kevin940726 commented Aug 17, 2021

getdave commented Aug 23, 2021

getdave commented Jul 21, 2021 •

edited

Loading

getdave commented Jul 29, 2021 •

edited

Loading