Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#14650 closed Bug (invalid)

classes on fake elements via createFakeParserElement not coming back properly.

Reported by: Tyler Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

Sorry, this is difficult to do "steps to complete". I encountered a bug doing custom plugin development - it relates to using a class as part of a fake object.

I created a custom plugin that overrides/enhances functionality of the anchor component of the link plugin. The only change I made was to add a custom className to created anchors, so they can work with a fixed header.

I took the existing anchor dialog pretty much verbatim - the only change was 'attributes' property the fake is created with now includes a class: 'anchor' declaration. I'm pretty sure this can be recreated by editing the attribtues in the anchor dialog to include class: "test", saving and refreshing.

https://github.com/ckeditor/ckeditor-dev/blob/67ae77b/plugins/link/dialogs/anchor.js#L27-L31

This works fine when inserting a fresh anchor however after reloading the page, the filter that runs as part of the "link" plugin tries to convert the html for anchors back to fakeobjects.

At this point, createFakeParserElement is called with the element, here: https://github.com/ckeditor/ckeditor-dev/blob/bd5101a/plugins/link/plugin.js#L184 ... at this point, element includes attributes which includes "name/id/data-cke-saved-name" and classes which includes "anchor".

The return value from this function is a fake object -- decoding the data-cke-realelement reveals that the real element does not have a class -- so performing a save at this point has the effect of stripping out the class name.

I was able to work around this with my own data filter rule that runs before the one in the link plugin - this will check for the presence of classes and if found, will add a "class" attribute to element.attributes - this seems to work fine, createFakeParserElement works fine if there is a class property in the attribute hash.

    editor.dataProcessor.dataFilter.addRules({
      elements: {
        a: function (element) {
          if (!element.attributes.name) { return null; }
          if (!element.children.length && element.classes.length) {
            element.attributes.class = element.classes.join(' ');
          }
          return null; // if we don't return, it passes along to the next filter.
        }
      }
    }, 9);

I think the bug here is that createFakeParserElement does not respect 'classes' inside the element it is passed.

I encountered this issue on 4.5.8 and am unsure if it is recreatable in other versions.

Change History (5)

comment:1 Changed 4 years ago by Jakub Ś

Resolution: invalid
Status: newclosed
Version: 4.5.8

First of all please ask your custom implementation questions on Stack Overflow in the future.


Please note that fake element is created with this method https://github.com/ckeditor/ckeditor-dev/blob/67ae77b/plugins/link/dialogs/anchor.js#L16. Now, the class for fake element is not passed in attributes and it is handled separately. If you want to change how anchor looks like, you can either change cke_anchor class in css files or you can create new plugin.


If you want this class to be present on real anchor element then this is doable.

  1. Add your custom class:
    'data-cke-saved-name': name,				
    'class' : 'test'
    
  2. Extend ACF so that class 'test' is accepted - https://github.com/ckeditor/ckeditor-dev/blob/bd5101a/plugins/link/plugin.js#L65
    editor.addCommand( 'anchor', new CKEDITOR.dialogCommand( 'anchor', {
    	allowedContent: 'a[!name,id](test)',
    
Version 0, edited 4 years ago by Jakub Ś (next)

comment:2 Changed 4 years ago by Tyler

Thanks for the response. I tried your suggestion of modifying the ACF filter -- I did it directly in my ckeditor.js bundle -- after including class, it did the same thing.

To clarify:

The initial creation of the fake element is fine, and the initial save is fine. It's only after it comes back from a refresh that the class gets stripped out.

The fake element that comes out of createFakeParserElement doesn't include the class, even though the parsed element does include the class attribute.

I'm not sure but this seems like a bug to me... I could ask in SO, but I'm not sure if that would be much help. Right now my workaround is fine, but I thought you'd all like to know of this problem.

comment:3 Changed 4 years ago by Jakub Ś

This is not a workaround. You are trying to change how this plugin works and you didn't researched it enough thus the problem.

CKEditor has nothing to do with saving and restoring Data. This is JS application. Only if you are able to reproduce this problem by switching to source mode and back (same actions are executed when getting and loading data), this can be considered CKEditor issue. From what I have checked classes were lost by changing modes but as I have explained already - this is because, by default anchor doesn't report any classes to Advanced Content Filter. You have simply forgotten about adjusting ACF.

Please also note that this is one case and there might be other cases when this class needs to be added to fake anchor or protected. Since you are changing default plugin behaviour, this is something you need to check on your own.

To learn about ACF please see:
http://docs.ckeditor.com/#!/guide/dev_acf
http://docs.ckeditor.com/#!/guide/dev_advanced_content_filter
http://docs.ckeditor.com/#!/guide/dev_disallowed_content
http://docs.ckeditor.com/#!/api/CKEDITOR.filter-method-addTransformations
http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-allowedContent
http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-extraAllowedContent

comment:4 Changed 4 years ago by Tyler

Hi thabks again for replying

I did try to modify the ACF filter for the command as you suggested and the same thing happened. There is no real problem from my end -- I can modify the element in the data processor rule to add attributes.class and it comes through properly, regardless of the ACF filter.

Thanks for your time though. If this is not considered a bug I'm ok with that because the workaround I posted above works fine.

comment:5 Changed 4 years ago by Jakub Ś

@tswaters yes you are right, sorry for that. I have forgotten that you also would need to modify this requiredContent: 'a[name](test)'. With that setting when you move to source and back, class test will not be removed.

You can also use the wildcard for classes:

editor.addCommand( 'anchor', new CKEDITOR.dialogCommand( 'anchor', {
	allowedContent: 'a[!name,id](*)',
	requiredContent: 'a[name](*)'

I have edited my previous post.

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