New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TINY-10892: Fix deleting from P with IMG into LI would create unwanted font-styles on Chrome #9638
Conversation
…d font-styles on Chrome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll review it when the unnecessary format changes are reversed.
modules/tinymce/src/core/test/ts/webdriver/delete/QuirksChromeTest.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrew Herron <thespyder@programmer.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix adds some weird undo levels. I really should have looked into this earlier since I think we just need to add a ElementType.isListItem
here: https://github.com/tinymce/tinymce/blob/main/modules/tinymce/src/core/main/ts/delete/BlockMergeBoundary.ts#L60 that is whats preventing the the merge blocks code from working with lists. There might be issues with merging list items into list items since some of that is overrided in the lists plugin so that you would outdent the list instead. But a complete override is better than trying to let the browser do it's thing, have a bunch of tracking states on that easier to test and easier to predict the behavior.
Co-authored-by: spocke <spocke@moxiecode.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well! That was quite a lesson in learning what the editor is capable of before building something that already exists 😂
I'm not sure I knew it existed either, to be fair.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's no longer really a chrome test, nor it is a quirks test 🤔
I'm happy to keep the test, just rename it (and remember the describe call includes the name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw #9648. We don't need two PRs for the same fix, as release/7
will be merged to main
after release.
Closing this PR as #9648 is the PR that should have existed to begin with. |
Related Ticket: TINY-10892
Description of Changes:
Pre-checks:
feature/
,hotfix/
orspike/
Review:
Docs ticket created (if applicable)GitHub issues (if applicable):