Ckeditor5: Style attribute conversion does not work with attributeToAttribute()

[Reinmar]

        editor.model.schema.extend( 'tableCell', {
            allowAttributes: 'style'
        } );

        editor.conversion.attributeToAttribute( {
            model: {
                name: 'tableCell',
                key: 'style'
            },
            view: 'style'
        } );

---

If you'd like to see this bug fixed sooner, add 👍 to this post.

[Reinmar]

Workaround for now:

        editor.model.schema.extend( 'tableCell', {
            allowAttributes: 'style'
        } );

        editor.conversion.for( 'upcast' ).attributeToAttribute( {
            model: {
                name: 'tableCell',
                key: 'style'
            },
            view: 'style'
        } );

        editor.conversion.for( 'downcast' ).add( dispatcher => {
            dispatcher.on( `attribute:style:tableCell`, ( evt, data, conversionApi ) => {
                const viewElement = conversionApi.mapper.toViewElement( data.item );

                conversionApi.writer.setAttribute( 'style', data.attributeNewValue, viewElement );
            } );
        } );

[jodator]

@Reinmar so the fix is pretty straight forward but it would need to change the way how we treat the style (from the docs):

If key is 'style', value is an object with key-value pairs.

to at least allow passing string there as the class attribute allows similar:

If key is 'class', value can be a String or an array of Strings

But OTOH just overwriting style attribute is not enough. We should parse this string to extract key-value parts to not overwrite the existing style attribute but only to add/overwrite each style separately.

But I'm not 100% sure what's the expectation here. From one side some other converters might create styles on element but obviously such cath-all style converter will include them as well.

As such I'm divided on whether we should allow string for style or object only notation.

[josueexhume]

Is there any fix for this? It outputs like this for me?

style="background-color:red;0:b;1:a;2:c;3:k;4:g;5:r;6:o;7:u;8:n;9:d;10:-;11:c;12:o;13:l;14:o;15:r;16::;17:r;18:e;19:d;20:;;"

Why does it add this part?
0:b;1:a;2:c;3:k;4:g;5:r;6:o;7:u;8:n;9:d;10:-;11:c;12:o;13:l;14:o;15:r;16::;17:r;18:e;19:d;20:;;

[jodator]

@josueexhume This is because the attributeToAttribute() converter expects the value for style key to be an object. This was done because some features might want convert only one style entry to model value. For instance the font family or font color are converting some styles while other are left untouched.

Right not the solution for this is to write a bit more specialized converters for style attribute like in previous comment. The general conversion helpers does not allow to convert whole style attribute.

[josueexhume]

So there's no way to allow converting the whole style attribute?, I really need that feature for my use case. Right now i have legacy html content, and i need to preserve the style attribute as i'm slowing fading it out in preference of applying styling based on a given class.

I thought maybe setting it up like this would of fixed my issue.

        conversion.for('upcast').attributeToAttribute({
            model: {
                name: 'div',
                key: 'style'
            },
            view: 'style'
        });

        conversion.for('downcast').add(dispatcher => {
            dispatcher.on('attribute:style:div', (evt, data, conversionApi) => {
                const viewElement = conversionApi.mapper.toViewElement(data.item);

                conversionApi.writer.setAttribute('style', data.attributeNewValue, viewElement);
            });
        });

[jodator]

You can also generalise this to all elements:

dispatcher.on('attribute:style', ... );

but adding :div is a safer option.

[scofalik]

Did you allow style attribute in model? The code you provided should work. I checked it for paragraph and it worked. Here's my code - I modified your snippet:

editor.model.schema.extend( 'paragraph', { allowAttributes: '__style' } );

editor.conversion.for('upcast').attributeToAttribute({
    model: {
        key: '__style',
        name: 'paragraph'
    },
    view: 'style'
});

editor.conversion.for('downcast').add(dispatcher => {
    dispatcher.on('attribute:__style:paragraph', (evt, data, conversionApi) => {
        const viewElement = conversionApi.mapper.toViewElement(data.item);

        conversionApi.writer.setAttribute('style', data.attributeNewValue, viewElement);
    });
});

[josueexhume]

@jodator thanks for the tip, i removed the :div.

@scofalik Thanks for the quick reply. I have the attribute allowed in another file, I tried your solution but it still appends a char array of the css style
<p style="color:red;0:c;1:o;2:l;3:o;4:r;5::;6:r;7:e;8:d;9:;;">hello</p>
Is there another part I'm missing?

[scofalik]

This is weird because it certainly works for me:

Are you sure you don't have an old code (converter) specified somewhere? Or some custom code which could affect this too? It kind of looks like this conversion was handled twice: once correctly and once incorrectly.

Try changing downcast converter to this:

editor.conversion.for('downcast').add(dispatcher => {
    dispatcher.on('attribute:__style:paragraph', (evt, data, conversionApi) => {
        conversionApi.consumable.consume( data.item, evt.name );

        const viewElement = conversionApi.mapper.toViewElement(data.item);

        conversionApi.writer.setAttribute('style', data.attributeNewValue, viewElement);
    }, { priority: 'highest' } );
});

And let me know if this helped. I've added higher priority (so it will be fired as a first converter) and consumed a consumable value (something you should do in order to prevent double conversion).

[josueexhume]

@scofalik
It works now, thank you very much for you all help, you and your team.

The mistake i was making was that i had i was declaring this in my widget for allowing divs. I moved it to another file (ie, plugin) where I'm enabling attributes and it worked perfectly. Just in case someone might run into the same issue here's my class.

off-topic: would there be an easier way to allow all attributes besides declaring each one?

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Div from './plugin/div/div';
import Section from './plugin/section/section';

export default class Extension extends Plugin {
    static get requires() {
        return [Div, Section];
    }

    init() {

        const editor = this.editor;

        let allowedAttributes = [
            'width', 'style', 'target', 'class', 'id', 'name', 'title', 'type', 'olddisplay', 'align', 'lang', 'dir',
            'border', 'cellspacing', 'cellpadding', 'color', 'valign', 'clear', 'src', 'height', 'shapes', 
            'prefix', 'tagtype', 'datetime', 'cite',  'cols', 'colspan', 'noshade',  
             'data-ingestor', 'shape', 'start', 'bgcolor', 'alt', 'strong', 'onclick', 'files',
            'com',  'utc-timestamp', 'eastern-timestamp', 'rowspan', 'span', 'theslate', 'page', 'content',
            'http-equiv', 'action', 'method', 'value', 'autofocus', 'maxlength', 'rows', 'for', 'aria-label', 'checked', 'selected',
            'rel', 'scope', 'location', 'cellpacing', 'block-id', 'guid',
            'data-pubtitle', 'data-paygoid', 'data-docid', 'nowrap', 
            'original-style', 'datatype', 'property', 'controls', 'controlslist', 'data-attr', 'poster', 'preload', 'str', 'itemprop',
            'ng-repeat', 'ng-if', 'tabindex', 'role', 'aria-describedby', 'aria-disabled',
            'aria-haspopup', 'onmouseover', 'onmouseout', 'onmouseup', '_cke_focus', 'hotel-location', 'office-location', 'xsd', 'xsi',
            'href_cetemp', 'uie-tracker', 'uie-tracking', 'track-download', 'track-trigger', 'col', 
            'doc', 'attach', 'pls', 'vspace', 'hspace', 'slatepath'
        ];

        editor.model.schema.extend('$root', { allowAttributes: allowedAttributes });
        editor.model.schema.extend('$block', { allowAttributes: allowedAttributes });
        editor.model.schema.extend('$text', { allowAttributes: allowedAttributes });
        editor.model.schema.extend('section', { allowAttributes: allowedAttributes });
        editor.model.schema.extend('div', { allowAttributes: allowedAttributes });

        for (var i = 0; i < allowedAttributes.length; i++) {
            editor.conversion.attributeToAttribute({ model: allowedAttributes[i], view: allowedAttributes[i] });
        }


        editor.model.schema.extend('paragraph', { allowAttributes: '__style' });

        editor.conversion.for('upcast').attributeToAttribute({
            model: {
                key: '__style',
                name: 'paragraph'
            },
            view: 'style'
        });

        editor.conversion.for('downcast').add(dispatcher => {
            dispatcher.on('attribute:__style:paragraph', (evt, data, conversionApi) => {
                conversionApi.consumable.consume(data.item, evt.name);

                const viewElement = conversionApi.mapper.toViewElement(data.item);

                conversionApi.writer.setAttribute('style', data.attributeNewValue, viewElement);
            });
        });
    }
}

Thank you again for all the help. Much appreciated.

[scofalik]

In such a case you can declare a schema rule through a callback:

this.editor.model.schema.addAttributeCheck( ( context, attributeName ) => {
    if ( allowedAttributes.includes( attributeName ) ) {
        return false;
    }
} );

The above should work. _I edited the snippet cause it used the event instead of addAttributeCheck(). Now it should be more correct._

More info here: https://ckeditor.com/docs/ckeditor5/latest/api/module_engine_model_schema-Schema.html#function-addAttributeCheck

[jodator]

Note about fix implementation: we might check if we can improve things with supporting catch-all converters for style and convert only non-consumed style entires in upcast/downcast. This potentially allows some features to control some style entry (like font-family, margin-top) only and other to pass other style entries as a catch-all converter.

[josueexhume]

Is there a way to get it to work for textproxy items? For example, this would fail for textproxy items

editor.conversion.for('downcast').add(dispatcher => {
            dispatcher.on('attribute:__style:paragraph', (evt, data, conversionApi) => {
                conversionApi.consumable.consume(data.item, evt.name);

// this will fail for the anchor element since the data.item is not an element but a textproxy
                const viewElement = conversionApi.mapper.toViewElement(data.item);

                conversionApi.writer.setAttribute('style', data.attributeNewValue, viewElement);
            });
        });

[scofalik]

If you are dealing with those, you need to use mapper.toViewRange() and writer.wrap(). AFAIR, the model range is in data.range.

[jodator]

I can see three solutions to this:

1. Fast, working in one case probably will break other features.

Treat styles attribute differently in the downcast conversion and make it behave like other attributes - so set the value directly. Fixes the stringification artifact in the view. This is probably a no-go as it wouldn't play nice with other features (probably such converter would have to be defined with a priority higher than other, including default, converters.

1. Proper solution - parsing the style string in upcast and downcast conversion and treat each of them as separate style entry.

Downcast - requires us to export the parseInlineStyles() and use it to parse the styles string.

Upcast - this is a bit trickier - we should process only non-consumed view values. I didn't research this but it might be trickier than it sounds. Also, we might not gain much implementing this other than closing this ticket.

1. Better docs - add notes in proper places (like conversion.attributeToAttribute() that it is not compatible with style as a string. That developers should avoid this. Also, we should add the workaround to the docs.

[jodator]

ps.: this is how the parsing of style key might look like (added as a reference):

if ( viewElementDefinition.styles ) {
    if ( typeof viewElementDefinition.styles === 'string' ) {

        const parsed = new Map();
        // This util should be refactored:
        parseInlineStyles( parsed, viewElementDefinition.styles );

        for ( const styleKey of parsed.keys() ) {
            viewWriter.setStyle( styleKey, parsed.get( styleKey ), element );
        }
    } else {
        const keys = Object.keys( viewElementDefinition.styles );

        for ( const key of keys ) {
            viewWriter.setStyle( key, viewElementDefinition.styles[ key ], element );
        }
    }
}

[usama-ayub]

i saw all above comments they all are help me but can i apply style attribute in

• image tag
• table tag
• link tag
• li tag
• tr tag
• td tag
• th tag

<table style="width:100%">
  <tbody>
    <tr style="width: 40%">
      <td style="width: 40%">
        Text on column 1
      </td>
      <td style="width: 60%">
        Text on column 2
      </td>
    </tr>
  </tbody>
</table>

and secondly how can i apply nested elements like <div><span>click</span></div>

[Mgsy]

A small note about this issue. I tried to add the conversion for style attribute in list items. I've used the solution from this ticket and finished with something like this:

editor.model.schema.extend( 'listItem', {
        allowAttributes: 'listStyle'
} );

editor.conversion.for( 'upcast' ).attributeToAttribute( {
  model: {
       name: 'listItem',
       key: 'listStyle'
   },
   view: 'style'
 } );
editor.conversion.for( 'downcast' ).add( dispatcher => {
    dispatcher.on('attribute:listStyle:listItem', ( evt, data, conversionApi ) => {
        const viewElement = conversionApi.mapper.toViewElement( data.item );
        conversionApi.writer.setAttribute( 'style', data.attributeNewValue, viewElement );
    } );
}, { priority: 'highest' } );

However, it turned out that it doesn't work for <li> elements, while the same code works fine for paragraphs. So, I've modified the upcast conversion and added value property. After this change, style attribute on list items converts properly:

editor.conversion.for( 'upcast' ).attributeToAttribute( {
    model: {
          name: 'listItem',
          key: 'listStyle'
     },
     view: {
         name: 'li',
         key: 'style',
         value: /[\s\S]+/
    }
} );