Opened 9 years ago

Closed 9 years ago

#5926 closed Bug (fixed)

Read-only regions, copy context menu action should be available

Reported by: Lynne Kues Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.4
Component: General Version:
Keywords: IBM Cc:

Description

3.4 Beta

Run the code example. Select the read-only region. Select the text in the region. Invoke context menu. Copy action is disabled.

Attachments (3)

ckeditor-test.html (2.6 KB) - added by Lynne Kues 9 years ago.
5926.patch (801 bytes) - added by Sa'ar Zac Elias 9 years ago.
5926_2.patch (712 bytes) - added by Sa'ar Zac Elias 9 years ago.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by Lynne Kues

Attachment: ckeditor-test.html added

comment:1 Changed 9 years ago by Tobiasz Cudnik

Keywords: Confirmed added

This can't be reproduced in FF, but in Chrome and IE, the context menu's "Copy" is disabled (although CTRL+C works as expected on not-editable selection).

Changed 9 years ago by Sa'ar Zac Elias

Attachment: 5926.patch added

comment:2 Changed 9 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: confirmedreview

Manually checking if the range is valid.

comment:3 Changed 9 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Note that, as commented in the code, we're using the 'Cut' command to check the 'Copy' state. It's supposed to be related to a browser bug.

I've tracked the original ticket for that thing. It's #984.

So far, that change was ok, because it was valid to consider that the Copy state will be the same as the Cut state in an editable environment. But in fact, things are different with read-only regions.

I've simply changed the check to the use the "Copy" command, and things started working well. For performance, that would be the right solution for it.

But #984 states that this may cause bugs in some browser: IE and Safari. So, we must check whether this is true.

Finally, we should isolate the range check fix for the browsers that really need it, if any, using "Copy" for all browsers that support it properly.

Changed 9 years ago by Sa'ar Zac Elias

Attachment: 5926_2.patch added

comment:4 Changed 9 years ago by Sa'ar Zac Elias

Status: review_failedreview

I didn't see any problem using stateFromNamedCommand( 'Copy' ).

comment:5 Changed 9 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

Please have this one committed into the 3.4.x branch.

comment:6 Changed 9 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [5734].

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy