Opened 5 years ago

Closed 5 years ago

#11508 closed Bug (fixed)

DataProcessor processing nested (encoded) attributes

Reported by: Olek Nowodziński Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.3.3
Component: Core : Parser Version: 4.0
Keywords: Cc: wim.leers@…

Description

  1. Open replacebycode sample.
  2. Enable allowedContent: true
    CKEDITOR.replace( 'editor1', {
    	allowedContent: true
    } );
    
  3. Call
    CKEDITOR.instances.editor1.dataProcessor.toHtml( '<p><img data-foo="&lt;a href=&quot;XXX&quot;&gt;YYY&lt;/a&gt;" /></p>' )
    
    data-foo attribute contains encoded HTML:
    <a href="XXX">YYY</a>
    

Expected:

<p><img data-foo="&lt;a href=&quot;XXX&quot;&gt;YYY&lt;/a&gt;" /></p>

Actual:

<p><img data-foo="&lt;a data-cke-saved-href=&quot;XXX&quot;&gt;YYY&lt;/a&gt; href=&quot;XXX&quot;&gt;YYY&lt;/a&gt;" /></p>

What happened? Dataprocessor considered href=&quot;XXX&quot; as an attribute of <img> and tried to protect it, making a lot of mess. The RegExp responsible for that is not perfect

protectAttributeRegex = /\s(on\w+|href|src|name)\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|(?:[^ "'>]+))/gi;

|(?:[^ "'>]+) matches href=&quot;XXX&quot; as an unquoted attribute of <img> (this kind of attributes is allowed in HTML)


Another TC:

Paste

<p><img data-foo="&lt;a href=&quot;XXX&quot;&gt;YYY&lt;/a&gt;" /></p>

into source view and switch modes twice.

Change History (6)

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

Owner: set to Olek Nowodziński
Status: newassigned

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

Status: assignedreview

Pushed a fix to t/11508 (+tests).

Note: Although I didn't notice performance loss (quite contrary in fact, which is funny) it may still be an issue because changes in RegExp made it more general. Watch out.

comment:3 Changed 5 years ago by Wim Leers

Cc: wim.leers@… added

comment:4 Changed 5 years ago by Frederico Caldeira Knabben

I was able to notice a small decrease on performance on setDate with this code (between 5-10%), but it is hard to measure it for sure. Still it has little impact on the overall editor experience. If we really want to talk about performance, then better to try to not have that code at all, which is a much bigger task.

I don't see better ways to solve this problem though. I just pushed a minor simplification to the regex and I think the proposed solution is fine.

comment:5 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

git:abb150f landed in master (eb840d1 tests).

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