Opened 7 years ago

Closed 6 years ago

Last modified 6 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 7 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 6 years ago by Jakub Ś (previous) (diff)

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

Description: modified (diff)

comment:3 Changed 6 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 6 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 6 years ago by Jakub Ś

Version: 4.13.5.1

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

This is broken from the beginning - #6665.

comment:7 in reply to:  4 ; Changed 6 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 6 years ago by Piotrek Koszuliński

I pushed t/10298 branch on tests.

comment:9 in reply to:  7 Changed 6 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 6 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 6 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 6 years ago by Olek Nowodziński

Rebased branches on latest master.

comment:14 Changed 6 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 6 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 6 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

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

comment:17 Changed 6 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 6 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 6 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 (although, I think it is because in my plugin I was using the same ckeditor css naming convention and I can't) it should be mentioned in the documentation AFAIK

Last edited 6 years ago by giammin (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy