It would be great if the code can be changed from the styles attribute to the type attribute or add a form that allows the change base on preferences.

The reason i ask is because styles will only work under full html and not in restricted html, but type will work in all of them as long as the type attribute is in the allowed tag.

Thanks

Comments

wolfhowling created an issue. See original summary.

joarferme’s picture

Adding the type attribute to the ordered and unordered list could fix this issue.
Go to the CKEditor library \libraries\liststyle\dialogs\liststyle.js
On lines 63-66 and 158-161

if ( value )
element.setStyle( 'list-style-type', value );
else
element.removeStyle( 'list-style-type' );

Replace these lines, to add the attribute type

if ( value ){
element.setStyle( 'list-style-type', value );
for(var tkey in mapListStyle){
if (mapListStyle[tkey]==value)
element.setAttribute( 'type', tkey );
}
} else {
element.removeStyle( 'list-style-type' );
element.removeAttribute( 'type' );
}

osman’s picture

Assigned: Unassigned » osman
Status: Active » Postponed (maintainer needs more info)

Any text format with Limit allowed HTML tags and correct faulty HTML filter enabled will strip the style attribute from ANY tags. See FilterHtml class:

The 'style' and 'on*' ('onClick' etc.) attributes are always forbidden, and are removed by Xss::filter()

However, the CKEditor List Style plugin sets the list properties via inline styles.

Unfortunately this is the reason why Basic HTML and Restricted HTML filter types cannot use this feature, at least for the time being.

I explored some workarounds to the issue to no avail. Patches and suggestions are welcome.

adamcadot’s picture

Version: 8.x-1.1 » 8.x-1.x-dev
Assigned: osman » Unassigned
Status: Postponed (maintainer needs more info) » Active
StatusFileSize
new1.22 KB

Posting a patch based on joarferme's suggestion.

Applies to the recommended version (4.8.0) and the latest version (4.11.1) of the liststyle plugin.

Note: The patch must be in the liststyle directory. (Typically at /libraries/ckeditor/plugins/liststyle)

Going forward, perhaps we should notify users that this patch exists by adding a warning to the module page. (Especially since the Limit allowed HTML tags and correct faulty HTML filter is a commonly used.)

piotrkonefal’s picture

Thanks @adamcadot. The patch works fine! After patching the library it is required to add <ol type> to allowed HTML tags if Limit allowed HTML tags and Correct faulty HTML filters are enabled.

I am using composer for managing dependencies, and composer-patches library so I didn't have to put the patch inside /libraries/ckeditor/plugins/liststyle.
I just added new patch entry to composer-patches.json file:

"ckeditor/liststyle": {
      "2874952: Change code from styles to type": "https://www.drupal.org/files/issues/2019-01-02/ckeditor_liststyle-styles-to-type-2874952-4-do-not-test.patch"
    }
b_sharpe’s picture

Status: Active » Needs work

Patch above works; however, it should be noted that the <ul> element is deprecating the "type" attribute in favor of CSS. The <ol> appears to be keeping it.

So at some point an alternate solution will need to be found.

bserem’s picture

Status: Needs work » Needs review

Please see the note here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/ol

Note: Unless the type of the list number matters (like legal or technical documents where items are referenced by their number/letter), use the CSS list-style-type property instead.

For format with "Limit HTML" it is should be enough to add this in the allowed html tags:
<ul list-style-type type> <ol list-style-type type start>

@osman you could update the documentation with the above line. I'm in the process of finding a way to automatically apply changes like that on hook_install, my approach is like this:

 if ($filter = FilterFormat::load('basic_html')) {
    if (($filter_html = $filter->filters('filter_html')) && $filter_html->status) {
      $config = $filter_html->getConfiguration();
      $allowed_html = !empty($config['settings']['allowed_html']) ? $config['settings']['allowed_html'] : NULL;
      $matches = [];
      if ($allowed_html && preg_match('/<ol ([^>]*)>/', $allowed_html, $matches)) {
        $new_attributes = array_filter(explode(' ', $matches[1]));
        $new_attributes[] = 'list-style-type';
        $config['settings']['allowed_html'] = preg_replace('/<ol ([^>]*)>/', '<div ' . implode(' ', array_unique($new_attributes)) . '>', $allowed_html);
        $filter->setFilterConfig('filter_html', $config);
        $filter->save();

      }
   }
 }

But it is still WIP and doesn't work as expected if the is already some configuration in the tag you are targetting.
I am setting this to needs review in the sense that the patch is not needed but a documentation change should solve the issue.

bserem’s picture

Status: Needs review » Needs work

Cancel my previous comment, I spoke too fast. CKEditor config allows:
<div class="mystyle--*">
but does not allow:
<ul style="list-style-type:*;" type> <ol style="list-style-type:*;" type start>

It does allow:
<ul type> <ol type start>
however...

Need to research further.

osman’s picture

Status: Needs work » Closed (outdated)