-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix for bookmarks created inside temporary elements #3487
Conversation
Since the CI was red, I have extracted the refactor commit to |
Seems like this breaks clipboard operations for widgets in IE8 - cutting, copying and pasting (dragging works fine). |
I've slightly modified |
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.
LGTM 👍
Unit tests
- Chrome (CI) ✅
- Firefox (CI) ✅
- Safari (dev & build) ✅✅
- IE8 (build) ✅
- IE11 (dev & build) ✅✅
- Edge (dev & build) ✅✅
Manual tests
With help of @msamsel and @Dumluregn 👍
- Chrome ✅
- Firefox ✅
- Safari ✅
- IE8 🚨 (issue with copy/paste, see - Fix for bookmarks created inside temporary elements #3487 (comment))
- IE9 ✅
- IE10 ✅
- IE11 ✅
After copypin/pastebin fix:
Unit tests
- Chrome (CI) ✅
- Firefox (CI) ✅
is:unit,path:/tests/plugins/widget,path:/tests/plugins/uploadwidget,name:bookmarks,path:/tests/core/dom,path:/tests/core/selection
- Safari (dev) ✅
- IE8 (dev) ✅
- IE9 (dev) ✅
- IE10 (dev) ✅
- IE11 (dev) ✅
- Edge (dev) ✅
Manual tests
is:manual,name:clipboardhtml,name:clipboardhtmlselectall,name:embed
- Chrome ✅
- Firefox ✅
- Safari ✅
- IE8 ✅
- IE9 ✅
- IE10 ✅
- IE11 ✅
- Edge ✅
What is the purpose of this pull request?
Bug fix
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What is the proposed changelog entry for this pull request?
What changes did you make?
I've forced bookmarks to be created outside temporary elements (elements with
[data-cke-temp=1]
). Thanks to that, widgets can now always restore bookmarks after copying.I've also refactored
CKEDITOR.dom.range#createBookmarks()
by deduplicating logic connected with creating bookmark nodes. However this commit can be extracted to separate PR.Closes #3423.
Closes #3482.