Opened 6 years ago

Closed 5 years ago

#11011 closed Bug (fixed)

Method applyStyle removes attributes from nested elements.

Reported by: Jakub Ś Owned by: Piotr Jasiun
Priority: Must have (possibly next milestone) Milestone: CKEditor 4.3.1
Component: General Version: 3.0
Keywords: Support Cc:

Description

  1. Please put attached file in samples folder (code for v3 is commented)
  2. Select 'some' and apply term style with button
  3. Select 'is some sample' and apply quote style with button
  4. Switch to source

Result: data-element="term" is removed

<span class="quote" data-element="quote">is <span class="term">some </span>sample</span>

Problem can be reproduced in every browser from CKEditor 3.0 in both CKE 3.x and 4.x

Attachments (1)

api2.html (3.1 KB) - added by Jakub Ś 6 years ago.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by Jakub Ś

Attachment: api2.html added

comment:1 Changed 6 years ago by Jakub Ś

Status: newconfirmed

comment:3 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.3.1

comment:4 Changed 6 years ago by Jakub Ś

Keywords: Support added

comment:5 Changed 5 years ago by Piotrek Koszuliński

We need to define a rule of thumb for this. By default we're removing duplicated attributes in nested elements. Only data-* attributes will be an exception?

comment:6 Changed 5 years ago by Piotrek Koszuliński

Priority: NormalHigh

comment:7 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:8 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

I have added the rule to removeOverrides to avoid removing data-* attributes. I have no ideas about others attributes we should thread the same way, I believe data-* attributes are special.

Changes in t/11011 and corresponding test branch.

comment:9 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  1. Wouldn't indexOf be better than substr?
  2. It would be good to have a test that even though data-* attr wasn't removed, the next one after it was, so we'll know that loop wasn't terminated completely.

comment:10 in reply to:  9 Changed 5 years ago by Piotr Jasiun

Status: review_failedreview

Replying to Reinmar:

  1. Wouldn't indexOf be better than substr?

No. indexOf is slower, because if it does not find 'data-' at the beginning it will look for match. Anyway I replaced substr with slice because the second one is standardized.

  1. It would be good to have a test that even though data-* attr wasn't removed, the next one after it was, so we'll know that loop wasn't terminated completely.

Good idea. Done.

Last edited 5 years ago by Piotr Jasiun (previous) (diff)

comment:11 Changed 5 years ago by Olek Nowodziński

Status: reviewreview_passed
  • Rebased both branches.
  • Pushed one very minor commit to each branch.

comment:12 Changed 5 years ago by Piotr Jasiun

comment:13 Changed 5 years ago by Piotr Jasiun

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