Opened 9 years ago
Closed 9 years ago
#13872 closed Bug (fixed)
Read-only mode allows cut
Reported by: | egli | Owned by: | Tomasz Jakut |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.5.5 |
Component: | General | Version: | 4.5.0 |
Keywords: | Cc: |
Description
Steps to reproduce
- Load CKEditor read-only, http://ckeditor.com/demo#read-only
- Select text
- Ctrl-X or Command-X
- Ctrl-Z or Command-Z
Expected result
- During cut, nothing should happen, or text should be placed on clipboard without being removed.
- Undo should have no effect.
Actual result
- Cut removes text and puts in on the clipboard.
- Undo exits out of read-only mode.
Other details (browser, OS, CKEditor version, installed plugins)
Tested on Chrome/Linux, Chrome/Mac, Safari/Mac.
Change History (8)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Status: | new → confirmed |
---|---|
Version: | 4.5.4 → 4.5.0 |
Problem can be reproduced from CKEditor 4.5.0 in all browsers.
comment:3 Changed 9 years ago by
Milestone: | → CKEditor 4.5.5 |
---|
comment:4 Changed 9 years ago by
Owner: | set to Tomasz Jakut |
---|---|
Status: | confirmed → assigned |
comment:5 Changed 9 years ago by
Status: | assigned → review |
---|
There was missing check in clipboard plugin if the editor is in read-only mode. Pushed fix to branch:t/13872
comment:6 Changed 9 years ago by
Status: | review → review_failed |
---|
- Now
CKEDITOR.plugins.clipboard.initPasteDataTransfer
will be called no matter what, resulting with changing your clippboard buffer if you'll do cut in readOnly mode.
The reason for that is that you're checking wrong event property
evt.type
where it should beevt.name
. I've found this one out while adding new tests.
- If I'm not mistaken this condition can be _shortened_ down to:
if ( !editor.readOnly || evt.name != 'cut' ) {
I've emphasized shortened, because I can't tell if it's equally easy to understand at first glance. We should go with easier to understand one, it's up to you to pick easier one.
- Please update manual tests accordiginly for testser also to ensure that nothing got copied to the clipboard.
I've pushed some my changes to branch:t/13872, make sure to pull them.
comment:7 Changed 9 years ago by
Status: | review_failed → review |
---|
1, 2. Yes, you're right.
- Updated.
I also updated unit tests because they were failing in IE. Pushed changes to branch:t/13872.
comment:8 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Merged to master with git:48311f8.
Actually, this seems to be Chrome only, and does not affect undo. I had loaded the wrong demo when testing against Safari.