Opened 11 years ago

Closed 11 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 Ś 11 years ago.

Download all attachments as: .zip

Change History (13)

Changed 11 years ago by Jakub Ś

Attachment: api2.html added

comment:1 Changed 11 years ago by Jakub Ś

Status: newconfirmed

comment:3 Changed 11 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.3.1

comment:4 Changed 11 years ago by Jakub Ś

Keywords: Support added

comment:5 Changed 11 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 11 years ago by Piotrek Koszuliński

Priority: NormalHigh

comment:7 Changed 11 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:8 Changed 11 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 11 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 11 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 substrwith 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.

Version 1, edited 11 years ago by Piotr Jasiun (previous) (next) (diff)

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

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

comment:12 Changed 11 years ago by Piotr Jasiun

comment:13 Changed 11 years ago by Piotr Jasiun

Resolution: fixed
Status: review_passedclosed
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