Opened 13 years ago

Closed 13 years ago

#8342 closed Bug (fixed)

ReadOnly sample - paste buttons reenabled, when "make read only" pressed

Reported by: Krzysztof Studnik Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.2
Component: UI : Toolbar Version: 3.6.2
Keywords: WebKit Cc:

Description

Safari/Mac

TC

  • go to sample ReadOnly
  • press "make read only" button : Editing is disabled, Copy/paste buttons are disabled
  • click editor body, and press some random keys on keyboard

Result

Paste as text, paste, paste from word buttons on toolbar are reenabled. When paste from word button is pressed, dialog window opens and user is able to paste ms word content. Text, pasted into dialog window, is not pasted to editor body.

Expected

Operations done on editor body, while read only mode is enabled, should not have influence on toolbar. Disabled buttons should stay disabled.

Attachments (4)

8342.patch (611 bytes) - added by Sa'ar Zac Elias 13 years ago.
8342_2.patch (1.4 KB) - added by Garry Yao 13 years ago.
8342_3.patch (1.2 KB) - added by Garry Yao 13 years ago.
8342_4.patch (602 bytes) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 13 years ago by Sa'ar Zac Elias

Keywords: WebKit added; Safari removed
Status: newconfirmed

Regression of this version, also in Chrome.

comment:2 Changed 13 years ago by Frederico Caldeira Knabben

Caused by [7242].

comment:3 Changed 13 years ago by Frederico Caldeira Knabben

Keywords: WebKit removed

Confirmed in all browsers.

comment:4 Changed 13 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.2

comment:5 in reply to:  3 Changed 13 years ago by Frederico Caldeira Knabben

Keywords: WebKit added

Replying to fredck:

Confirmed in all browsers.

My fault... that's WebKit only.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 8342.patch added

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

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

Since I didn't find anything else that doesn't work.. I think it's safe to just check read only mode to avoid problems.

comment:7 Changed 13 years ago by Garry Yao

Keywords: NeedsTest added
Status: reviewreview_failed

Not all clipboard commands are to be excluded from readonly, e.g. "copy". Please add a automatic test for it.

Changed 13 years ago by Garry Yao

Attachment: 8342_2.patch added

comment:8 Changed 13 years ago by Garry Yao

Owner: changed from Sa'ar Zac Elias to Garry Yao
Status: review_failedreview

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

Status: reviewreview_failed
  • Tests don't pass with that patch in IE and FF.
  • Noneditable selections are not being considered with the patch, e.g.
    <p contenteditable="false">aa^aa</p>
    

Note that the copy command is anyway disabled in WebKit, not due to the patch.

Last edited 13 years ago by Sa'ar Zac Elias (previous) (diff)

comment:10 Changed 13 years ago by Garry Yao

Status: review_failedreview_passed

R+ for first patch, I thought wrongly last review.

comment:11 Changed 13 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [7267], I see that the tt was already updated.

comment:12 Changed 13 years ago by Garry Yao

Resolution: fixed
Status: closedreopened

After some investigation, there's a deeper reason behind this bug instead, which is caused by JS error on "selectionchange" event listeners.

Changed 13 years ago by Garry Yao

Attachment: 8342_3.patch added

comment:13 Changed 13 years ago by Garry Yao

Status: reopenedreview

comment:14 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

Nice catch @garry.yao.

comment:15 Changed 13 years ago by Garry Yao

Keywords: NeedsTest removed
Resolution: fixed
Status: review_passedclosed

Fixed with [7269].

Changed 13 years ago by Garry Yao

Attachment: 8342_4.patch added

comment:16 Changed 13 years ago by Garry Yao

Resolution: fixed
Status: closedreopened

As previous commit doesn't pass the tt in all browsers.

comment:17 Changed 13 years ago by Garry Yao

Status: reopenedreview

comment:18 Changed 13 years ago by Garry Yao

Resolution: fixed
Status: reviewclosed

Yet another the review reveals it a fabricated problem created by the tc instead of a real bug, the patch is obsoleted after tc is updated.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy