Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#10298 closed Bug (fixed)

Data processor breaks *-href attributes on links

Reported by: Piotrek Koszuliński Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 4.1.2
Component: General Version: 3.5.1
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

Reported on SO: http://stackoverflow.com/questions/15795142/is-there-anyway-to-stop-ckeditor-4-from-filtering-my-anchor-tag-attributes

Open editor with disabled ACF.

Set data: <p><a href="#" data-href="x">foo</a></p>

Result: <p><a data-="" href="#">foo</a></p>

All link attributes ending with /[^a-z]href are broken

Change History (20)

comment:1 Changed 11 years ago by Jakub Ś

Status: newconfirmed
Version: 4.1

Problem can be reproduced in all browsers from CKEditor 3.5.1 in both CKE 3.x and 4.x branches.

Last edited 11 years ago by Jakub Ś (previous) (diff)

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

Description: modified (diff)

comment:3 Changed 11 years ago by Oskar Schöldström

Some additional information:

  • Using the Drupal CKEditor module with version 4.1.1, the editor is able to save the attribute correctly, but strips it once the WYSIWYG is loaded.
  • Still happens when all plugins except for wysiwygarea are disabled.

comment:4 Changed 11 years ago by Oskar Schöldström

I submitted a pull request on github. https://github.com/ckeditor/ckeditor-dev/pull/38

The issue is that a regex matching protected attributes uses a \b-modifier, and as the - character is not a word-character the full attribute name will get split at the dash.

comment:5 Changed 11 years ago by Jakub Ś

Version: 4.13.5.1

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

This is broken from the beginning - #6665.

comment:7 in reply to:  4 ; Changed 11 years ago by Piotrek Koszuliński

Replying to oxy:

I submitted a pull request on github. https://github.com/ckeditor/ckeditor-dev/pull/38

The issue is that a regex matching protected attributes uses a \b-modifier, and as the - character is not a word-character the full attribute name will get split at the dash.

Unfortunately this is not a correct patch. It breaks e.g.:

toHtml( <p><img onmouseout="this.src=\'out.png\'" onmouseover="this.src=\'over.png\'" src="http://t/image.png" /></p>

Expected:
<p><img data-cke-pa-onmouseout="this.src='out.png'" data-cke-pa-onmouseover="this.src='over.png'" data-cke-saved-src="http://t/image.png" src="http://t/image.png" /></p>
Actual:
<p><img data-cke-pa-onmouseout="this.src='out.png'" data-cke-pa-onmouseover="this.src='over.png'" data-cke-saved-="" src="http://t/image.png" /></p>

basic case like this:

toHtml( '<a\n href="http://ckeditor.com">foo</a>' )

Expected:
<a data-cke-saved-href="http://ckeditor.com" href="http://ckeditor.com">foo</a>
Actual:
<a data-cke-saved-="" href="http://ckeditor.com">foo</a>

and does not fix the issue:

toHtml( '<p><a data-href="x" href="#" src-foo="y">A</a></p>' )

Expected:
<p><a data-cke-saved-href="#" data-href="x" href="#" src-foo="y">A</a></p>
Actual:
<p><a data-cke-saved-="" data-href="x" href="#" src-foo="y">A</a></p>

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

I pushed t/10298 branch on tests.

comment:9 in reply to:  7 Changed 11 years ago by Oskar Schöldström

Replying to Reinmar:

Replying to oxy:

I submitted a pull request on github. https://github.com/ckeditor/ckeditor-dev/pull/38

The issue is that a regex matching protected attributes uses a \b-modifier, and as the - character is not a word-character the full attribute name will get split at the dash.

Unfortunately this is not a correct patch. It breaks e.g.:

toHtml( <p><img onmouseout="this.src=\'out.png\'" onmouseover="this.src=\'over.png\'" src="http://t/image.png" /></p>

Expected:
<p><img data-cke-pa-onmouseout="this.src='out.png'" data-cke-pa-onmouseover="this.src='over.png'" data-cke-saved-src="http://t/image.png" src="http://t/image.png" /></p>
Actual:
<p><img data-cke-pa-onmouseout="this.src='out.png'" data-cke-pa-onmouseover="this.src='over.png'" data-cke-saved-="" src="http://t/image.png" /></p>

basic case like this:

toHtml( '<a\n href="http://ckeditor.com">foo</a>' )

Expected:
<a data-cke-saved-href="http://ckeditor.com" href="http://ckeditor.com">foo</a>
Actual:
<a data-cke-saved-="" href="http://ckeditor.com">foo</a>

and does not fix the issue:

toHtml( '<p><a data-href="x" href="#" src-foo="y">A</a></p>' )

Expected:
<p><a data-cke-saved-href="#" data-href="x" href="#" src-foo="y">A</a></p>
Actual:
<p><a data-cke-saved-="" data-href="x" href="#" src-foo="y">A</a></p>

Thanks for the review Reinmar! Is the test branch available anywhere? I can't find an up-to-date version, nor can I get the current docs on CKTester to work.

comment:11 Changed 11 years ago by Oskar Schöldström

Oh, thanks for saving me some headache hehe.

I'll come back to this once you have released your test branch, otherwise I'll just be wasting your time.

comment:12 Changed 11 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.1.2
Owner: set to Frederico Caldeira Knabben
Status: confirmedreview

oxy's patch was almost good. Fix proposed in t/10298.

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

Rebased branches on latest master.

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

Status: reviewreview_passed
  • I found that there's one wrong, extra commit in test branch (4e06e7c96c362) that belongs to #10367.
  • Pushed MT for this ticket.

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

I recommend using slice() instead of substr(): ​http://jsperf.com/substr-and-substring-and-slice.

The performance is not an issue (it's nearly equal, isn't it?). Slice is better because it is standardised:

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

Resolution: fixed
Status: review_passedclosed

Merged fix into master, dev (​git:8ad21b6), tests (efae6db).

comment:17 Changed 11 years ago by giammin

this is not fixed!

i experience the same problem with

element.addClass('cke_blabla')
element.attributes.add('class','cke_blabla')
editor.insertHtml('<div class="cke_blabla">dfgdfg</div>')

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

@giammin - even if there is an issue (although, as j.swiderski explained, most likely there's not), it's not related to this specific ticket.

comment:20 in reply to:  19 Changed 11 years ago by giammin

Replying to Reinmar:

@giammin - even if there is an issue (although, as j.swiderski explained, most likely there's not), it's not related to this specific ticket.

if it is not an issue it should be mentioned in the documentation AFAIK

Version 0, edited 11 years ago by giammin (next)
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