Prevent paste_convert_word_fake_lists from removing content and detect roman numerals correctly#6621
Closed
aautio wants to merge 2 commits intotinymce:developfrom
Closed
Prevent paste_convert_word_fake_lists from removing content and detect roman numerals correctly#6621aautio wants to merge 2 commits intotinymce:developfrom
aautio wants to merge 2 commits intotinymce:developfrom
Conversation
Fixes tinymce#6620 Previously roman numerals with more than two characters caused the list to split into two. E.g. the list below would split to ol & ul at third item: i. First ii. Second iii. Third The included test case covers numerals from I to X.
Fixes tinymce#2810, Fixes tinymce#3017, Fixes tinymce#3480, Fixes tinymce#5474 This commit includes fixes to multiple subtle errors in MS Word list cleanups. The bugs caused any characters ending with a dot to disappear from the lists under certain circumstances: Font/style -changes cause multiple text nodes to appear as siblings in the AST. Function filterStyles marked only the first node with _listIgnore and thus missed the nodes with roman numerals. This was fixed. The removeIgnoredNodes-function used node.remove() to delete nodes from the AST. The method sets node.next to null and thus recursion did not proceed further. Sibling text nodes with _listIgnore would not be deleted. This was fixed by avoiding node.remove(). The trimListStart -function did not trim the beginning of the paragraph. It trimmed all text nodes instead and removed the ones with words ending with dot. This was fixed by mering the paragraph text nodes before trimming the first text node. Regex for cleaning up the beginning is now safe. It no longer removes any letters (with \w). Instead it only looks for certain bullets. One regex is enough to filter those as the text nodes have been merged before matching. Added a couple of test cases as a proof that the fixes work correctly.
Contributor
|
Hi @aautio, Thanks very much for your contribution, at a glance this looks good. We’d like if you could split this PR into two (one PR per commit) to shrink the scope a little and help with review. We’ll also need you to sign the CLA (you should have received an email). Thanks again! |
Contributor
Author
|
Hi, @JamesToohey! Sure, I'll split this PR to two tomorrow. CLA email did not arrive, could you resend it please? |
This was referenced Mar 31, 2021
Contributor
Author
This file contains hidden or 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
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.
Hi,
Here are fixes to the MS Word paste cleanup.
Please see the full commit messages for rationale and details about the changes.
I'll be happy to elaborate or edit the PR according to your feedback.
Pre-checks:
GitHub issues: