Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#10006 closed Bug (fixed)

Strip entire element after removing required attr/style

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.1 RC
Component: General Version:
Keywords: Drupal Cc: wim.leers@…

Description (last modified by Piotrek Koszuliński)

E.g. <a name=""></a> should be removed when name attribute was filtered out.

Similar case: currently <span> is allowed because plugins like font, colorbutton allow styles which base on span elements. So when pasting e.g. <span lang="pl">, lang is filtered out, but <span> left.

Proposed fix:

There should be a way to mark attrs/style/class as required. Element is marked by this rule as valid (element has to be valid, otherwise it will be filtered out) only if it has this required property.

Possible format:

"a[!href]; span{!color}"

PS. Plugins like font and colorbutton use styles to register content they generate, so by default when passing style to filter#allow() all parts of style should be marked as required.

Change History (12)

comment:1 Changed 7 years ago by Piotrek Koszuliński

Status: newconfirmed

comment:2 Changed 7 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:3 Changed 7 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:4 Changed 7 years ago by Piotrek Koszuliński

Summary: Strip entire inline element after removing required attr/styleStrip entire element after removing required attr/style

It doesn't make sense to restrict this to inline content only. I can't see any case in which block element without some property should be removed, but theoretically <a> as HTML5's transparent element may become a block one.

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

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

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

Status: assignedreview

Pushed t/10006 on dev and tests. Both branches are based on #9989.

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

Rebased branches on t/9989.

comment:8 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Merged to major on git:3a9b2c5 on dev and 33387db on tests.

comment:10 Changed 7 years ago by Wim Leers

Looks great.

And the syntax makes sense (cfr. CSS'!important).

comment:11 Changed 7 years ago by Piotrek Koszuliński

In fact !important means 'not important' :D. But I'm glad that you liked our syntax :).

comment:12 Changed 7 years ago by Wim Leers

That's a good point :)

But fact is of course that that is not what the CSS syntax is trying to convey :)

So, let me restate: from a "typical programming language POV" it makes no sense, but it's analogous to the CSS syntax, so it does makes sense in this context!

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