Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#10224 closed Bug (fixed)

ACF should not clean up non-empty link without href attribute

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.1
Component: General Version: 4.1
Keywords: Cc: info@…

Description

Source: http://ckeditor.com/comment/124493#comment-124493

I think that I got a little bit too far when implementing #10073. Non-empty link without href attribute is valid (in HTML5) and should not be stripped when allowed by ACF. Links (not including anchors - <a> elements with name and without href) should only be stripped when they are empty.

Attachments (1)

10224.html (691 bytes) - added by Piotrek Koszuliński 7 years ago.

Download all attachments as: .zip

Change History (10)

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

Status: newconfirmed

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

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

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

Status: assignedreview

Pushed t/10224 on dev and tests.

  • In validateElement I decided to use the same code which htmlDP uses. Thanks to that it will be backward compatible.
  • I had to add clean up for data-cke-saved-* attributes, because after stripping href, data-cke-saved-href was left, so href was recreated after getting data.

comment:4 Changed 7 years ago by Olek Nowodziński

Status: reviewreview_passed

Review passed. But non-trivial things like this in tests need additional explanation:

filter( '<p>X<a href="x" name=""></a>X</p>',     '<p>XX</p>' );

vs.

filter( '<p>X<a name="">A</a>X</p>',             '<p>X<a name="">A</a>X</p>' );

Otherwise we'll get lost soon.

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

Resolution: fixed
Status: review_passedclosed

Fixed on major with git:0948a5b on dev and b54618f on tests.

I pushed special comments explaining not obvious cases.

comment:6 Changed 7 years ago by Bert Hankes

Cc: info@… added

Hi,

Just installed 4.1 and still the same problem :-(

I have (exerpt):

var allowed = 'a(iLink)[!title,!onclick,href]', required = 'a(iLink)[title,onclick]';

editor.addCommand( 'internelink', new CKEDITOR.dialogCommand( 'internelinkDialog' ),{allowedContent: allowed, requiredContent: required } );

It produces the following tag: <p><a class="iLink" onclick="iLink(25)" title="test">test</a></p>

But when changing to source and back all left is: <p>test</p>

I could be wrong in programming the plugin, but I use the link-example as reference!

Last edited 7 years ago by Bert Hankes (previous) (diff)

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

I made a sample 10224.html that proves that ACF, if configured correctly, does not remove this link.

I think that the problem is somewhere else. You add a command, but you do not activate the feature. A feature is activated automatically only when it is a button added to toolbar (I don't see a button in that excerpt). In other cases you need to call editor.addFeature() manually. In your case it will be:

var cmd = editor.addCommand( 'interlink', ... );
editor.addFeature( cmd );

Check the "A button, command or maybe a plugin – which of them is a feature?" chapter in this guide: http://docs.ckeditor.com/#!/guide/plugin_sdk_integration_with_acf

Changed 7 years ago by Piotrek Koszuliński

Attachment: 10224.html added

comment:8 Changed 7 years ago by Bert Hankes

Hi Reinmar,

You made my day! Briljant.. it works!!!! Keep this ticket closed and you have earned a beer!

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

Hehe :) I'm glad I could help. And that this isn't an editor bug ;).

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