Fix pasting MS Word table #55206
Open
Fix pasting MS Word table #55206
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
ellatrix
added
[Type] Bug
|
Size Change: +11 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
|
Thanks for the PR! When I tried to paste from Excel, the following error appeared in the browser console. Therefore, I added a process to export |
t-hamano
reviewed
Oct 10, 2023
There was a problem hiding this comment.
This PR works correctly for me!
Copy from Google docs (spreadsheet)
- ✅ Normal table
- ✅ Table with merged cells
Copy from MS Word
- ✅ Normal table
- ✅ Table with merged cells
Others
I also tested whether the issue fixed in #33607 caused a regression.
- ✅ When you copy and paste inline text, there are no spaces at either end.
- ✅ If you intentionally copy and paste text that contains spaces, spaces will remain.
22b9b5ec4bb7919a3b329918ed12094b.mp4
I would be happy if this PR could address the following two things:
- Add raw-handling test (Like
ms-excel-table-in|out.htmlfor example?) - I think this PR will expose
getPasteEventData()function, so how about adding JSDoc so that it is reflected in the README?
|
Yes, I'll add a test, and I'll lock the function as a private api for now |
|
Flaky tests detected in d9c7457e0f684afc88f1f940a0e0939e84bba19c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6469903118
|
ellatrix
force-pushed
the
fix/ms-word-table-paste
branch
from
d9c7457
to
a50954a
Compare
|
This latest PR appears to re-introduce the issue fixed in #33607. 6c6919e68eda92b8c2269248d427a1ab.mp4For example, if I copy and paste the word "WordPress" from within a paragraph block, the HTML before filtering will look like this: It looks like it's probably because there's an extra text node before export default function msFragmentRemover( node ) {
if (
node.nodeType === node.COMMENT_NODE &&
[ 'StartFragment', 'EndFragment' ].includes( node.textContent.trim() )
) {
// Remove unnecessary text nodes before StartFragment or after EndFragment.
const textNode =
node.textContent.trim() === 'StartFragment'
? node.previousSibling
: node.nextSibling;
if ( textNode && textNode.nodeType === textNode.TEXT_NODE ) {
remove( textNode );
}
// Remove the fragment comment node.
remove( node );
}
} |
t-hamano
reviewed
Oct 11, 2023
Co-authored-by: Aki Hamano <[email protected]>
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.
What?
Fixes #42570.
Pasting tables (and potentially other stuff) from MS Word is broken because everything before and and after the StartFragment/EndFragment comments is stripped.
This is a regression caused by #33607.
Why?
How?
The solution is to reuse the battle tested strippers from the paste handler instead of doing something different.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast