Ticket #5455 (closed Bug: fixed)

Opened 4 years ago

Last modified 4 years ago

we can't remove the format of the copied & pasted text by selecting the text and clicking on Formatted icon

Reported by: satya Owned by: fredck
Priority: Normal Milestone: CKEditor 3.4
Component: Core : Styles Version: 3.0
Keywords: IBM Cc: damo,joek

Description

To reproduce the defect:

  1. Open Ajax sample.
  1. Type few lines of text.
  1. Apply Bold Formatting to the text in the First line.
  1. Copy the Formatted text in the first line and Paste it at the end of last line.
  1. Select the Pasted text and see that the Bold icon in Toolbar is highlighted.
  1. Click on Bold Icon.

Expected Result:

Bold Format applied to the text is removed

Actual Result:

Bold Format applied to the text is not removed.

We can remove the format using only the following two ways.

Go to HTML Source and Come back to WYSIWYG mode and select the text again and click Bold icon or

Select the Pasted text and Click on Remove Format Icon.

Same behaviour happens with Italic,Underline,Strike Through,Subscript and Superscript formatting options.

Tested against FF3,IE 6&7

Attachments

5455.patch (846 bytes) - added by brooks 4 years ago.
5455_2.patch (863 bytes) - added by brooks 4 years ago.
5455_3.patch (864 bytes) - added by brooks 4 years ago.
5455_4.patch (772 bytes) - added by garry.yao 4 years ago.
5455_5.patch (2.2 KB) - added by Saare 4 years ago.
5455_6.patch (833 bytes) - added by fredck 4 years ago.

Change History

comment:1 Changed 4 years ago by fredck

  • Keywords Confirmed added
  • Version changed from 3.2 to 3.0
  • Milestone changed from CKEditor 3.3 to CKEditor 3.4

Confirmed with CKEditor 3.0.

comment:2 Changed 4 years ago by brooks

  • Status changed from new to assigned
  • Owner set to brooks

comment:3 Changed 4 years ago by brooks

only reproduceable in FF

Changed 4 years ago by brooks

comment:4 Changed 4 years ago by brooks

  • Keywords review? added

Changed 4 years ago by brooks

Changed 4 years ago by brooks

comment:5 Changed 4 years ago by brooks

pls just review for 5455_3.patch

comment:6 Changed 4 years ago by garry.yao

  • Keywords Discussion review- added; Confirmed review? removed

The patch doesn't work when both attributes present on element.

On the other hands, I think our styles system is doing wrong in this sense, though we have kept it for a long time since v2, e.g. user put a class attribute on the strong element could easily break the style removing, which doesn't make sense.

Changed 4 years ago by garry.yao

comment:7 Changed 4 years ago by garry.yao

  • Component changed from General to Core : Styles

Proposing a solution to have it discussed.

comment:8 Changed 4 years ago by garry.yao

  • Keywords Review? added; review- removed

comment:9 Changed 4 years ago by fredck

  • Status changed from assigned to new
  • Owner changed from brooks to garry.yao

comment:10 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed

The proposed approach is wrong, because now we'll leave an empty <span> when removing the bold formatting, just because of the presence of the _cke_expando attribute.

We need to check where we're using element.hasAttributes(), but maybe we can make it ignore the _cke_expando attribute. In that case the change would be much simpler and safer.

Changed 4 years ago by Saare

comment:11 Changed 4 years ago by Saare

  • Keywords Discussion removed
  • Status changed from review_failed to review
  • Owner changed from garry.yao to Saare

The patch makes the hasAttributes method to ignore the _moz_dirty attribute, as well as adds Garry's solution.

comment:12 Changed 4 years ago by fredck

  • Status changed from review to review_failed

Having or not to handle removal of styles partially valid for the selection, replacing them with <span>, is something out of the scope of this ticket, and needs discussions, together with a real world test case.

comment:13 Changed 4 years ago by fredck

  • Status changed from review_failed to assigned
  • Owner changed from Saare to fredck

Changed 4 years ago by fredck

comment:14 Changed 4 years ago by fredck

  • Status changed from assigned to review

KISS

comment:15 Changed 4 years ago by Saare

  • Status changed from review to review_passed

Filed #6026 to discuss Garry's proposal.

comment:16 Changed 4 years ago by fredck

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [5741].

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