Skip to content

Prevent paste_convert_word_fake_lists from removing content and detect roman numerals correctly#6621

Closed
aautio wants to merge 2 commits intotinymce:developfrom
aautio:list_fixes
Closed

Prevent paste_convert_word_fake_lists from removing content and detect roman numerals correctly#6621
aautio wants to merge 2 commits intotinymce:developfrom
aautio:list_fixes

Conversation

@aautio
Copy link
Copy Markdown
Contributor

@aautio aautio commented Mar 14, 2021

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.
@JamesToohey
Copy link
Copy Markdown
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!

@aautio
Copy link
Copy Markdown
Contributor Author

aautio commented Mar 25, 2021

Hi, @JamesToohey! Sure, I'll split this PR to two tomorrow. CLA email did not arrive, could you resend it please?

@aautio
Copy link
Copy Markdown
Contributor Author

aautio commented Mar 31, 2021

I've submitted two pull requests (one per commit) as #6689 and #6690. Thus closing this one.

@aautio aautio closed this Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants