<span style> breaks 'Limit allowed HTML tags and correct faulty HTML' validation

Created on 4 May 2023, over 1 year ago
Updated 27 June 2024, 5 months ago

Problem/Motivation

It appears as if 'Limit allowed HTML tags and correct faulty HTML' fails to validate with the current and configuration as seen in the ckeditor_font configuration.

When removing and leaving to just , the functionality validates.

Can someone confirm that the 'Limit allowed HTML tags' breaks this validation on a text_format in CKEditor5?

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Needs work

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States Webbeh Georgia, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Webbeh
  • πŸ‡ͺπŸ‡ΈSpain bmunslow

    That's correct, filter_html filter is incompatible with the ckeditor_font pugins (font size and font family), because the style attribute is not allowed.

    See:

  • πŸ‡ΉπŸ‡­Thailand Mandras87

    I confirm the problem.
    What are the possible workarounds ?
    A clean during the paste ?
    Use data attributes ?
    Use class names ?

  • πŸ‡ͺπŸ‡ΈSpain bmunslow

    The most straightforward workaround is obviously to disable the limit_html filter.

    Sadly, as far as I know, it's not really possible to transform the style attribute to class names in this case (although it is doable with the color button plugin β†’ , for instance).

    The only option would be to develop your own custom font-size and font-family CKEditor5 plugins so they create and assign class names to style each element, instead of using the style attribute.

    CKEditor 5 Dev Tools β†’ would be a good starting point.

  • Here's a workaround which changes the font plugin to generate <span class="font-size-*"> instead of <span style="font-size: *px">. You also need to create corresponding CSS rules to apply the font size to the font-size-* classes.

    function hook_editor_js_settings_alter(array &$settings) {
      foreach ($settings['editor']['formats'] as &$format) {
        if (isset($format['editorSettings']['config']['fontSize']['options'])) {
          foreach ($format['editorSettings']['config']['fontSize']['options'] as &$option) {
            // Change the font size plugin to generate <span class="font-size-*">
            // instead of <span style="font-size: *px">.
            // Adding the 'view' property causes the font plugin to treat the option
            // as a "full definition".
            // See getOptionDefinition(), isFullItemDefinition(), and namedPresets
            // in ckeditor5-font/src/fontsize/utils.js
            $option['view'] = [
              'name' => 'span',
              'classes' => 'font-size-' . intval($option['model']),
              'priority' => 7,
            ];
          }
        }
      }
    }
    
    function hook_ckeditor5_plugin_info_alter(array &$plugin_definitions) {
      // Replace the <span style> element with <span class>, since we alter the
      // plugin to use classes instead of inline styles.
      $font_plugin_definition = $plugin_definitions['ckeditor_font_font']->toArray();
      $font_plugin_definition['drupal']['elements'] = ['<span>', '<span class>'];
      $plugin_definitions['ckeditor_font_font'] = new CKEditor5PluginDefinition($font_plugin_definition);
    }
    
  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    Shouldn't the system automatically add "style" as an allowed attribute for "span" as it's listed in the plugin's "elements" definition?

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Shouldn't the system automatically add "style" as an allowed attribute for "span" as it's listed in the plugin's "elements" definition?

    See #2 + #4.

    The style attribute is never allowed by filter_html. It’s an XSS-protection thing that cannot be disabled. It’s not related to CKEditor 5.

    What this does mean is that we should add even stricter validation to CKEditor 5 in Drupal core, which would inform the user that a particular plugin cannot be used while filter_html is enabled!

    Could you please create an issue for that, @DamienMcKenna? πŸ™

    (Note that all of these restrictions ALSO existed in CKEditor 4, Drupal 7/8/9, but they were just quietly ignored β€” CKEditor 5's validation logic tries to catch every single incompatibility. This is one we didn't think about πŸ˜….)

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I was wondering why this isn't a more widespread problem. I'd SWEAR we prevented this in the CKEditor 5 admin UI! πŸ˜…

    The answer:

    ckeditor5_table_properties:
    …
        conditions:
          plugins:
            - ckeditor5_table
            # When arbitrary HTML is already allowed, it's harmless to enable CKEditor 5's UI for table properties.
            - ckeditor5_arbitraryHtmlSupport
        elements:
          - <table style>
    

    and

    ckeditor5_table_cell_properties:
    …
        conditions:
          plugins:
            - ckeditor5_table
            # When arbitrary HTML is already allowed, it's harmless to enable CKEditor 5's UI for table cell properties.
            - ckeditor5_arbitraryHtmlSupport
        elements:
          - <td style>
          - <td rowspan colspan style>
          - <th style>
          - <th rowspan colspan style>
    

    These are the two only CKEditor 5 plugins in Drupal core that claim to support creating style attributes. Note that they both have
    a plugin condition like this:

            # When arbitrary HTML is already allowed, it's harmless to enable CKEditor 5's UI for table cell properties.
            - ckeditor5_arbitraryHtmlSupport
    

    That works in those cases, because they're automatically enabled enhancements for a manually enabled plugin/toolbar item ("Table").

    But this is a different case. In this case, we need to have a validation error triggered whenever filter_html is enabled and we're trying to add this plugin by adding a toolbar item.

    On top of that, we need to expand the validation logic in \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::validateDrupalAspects() to inform plugin developers that whenever they specify the style attribute, that ckeditor5_arbitraryHtmlSupport MUST be explicitly listed as a plugin condition.

    Once those 2 things are done in Drupal core, this confusing behavior will no longer be possible, and the UX will be great again πŸ€“

  • Status changed to Needs review about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    So something like this?

  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    The bug here is that the plugin and UI need to direct the site builder to configure the site appropriately.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Actually, there may be a simpler solution that satisfies more people:

    1. many people want to use this plugin's functionality (e.g. set line height/font size)
    2. WITHOUT using "Full HTML"

    https://www.drupal.org/project/extended_html_filter β†’ was created for literally this purpose: it's filter_html, but with the style attribute allowed.

    It was still a bit rough around the edges though, so created a MR to smoothen that out: #3401513-3: Match the filter_html <> ckeditor5 integration in Drupal core β†’ .

  • heddn Nicaragua

    On 10.2+, the additional validation for ck5 attributes and properties is getting in the way. I attempted to use extend_html_filter and the the MR from ✨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work and still no joy.

  • Status changed to Needs work 11 months ago
  • heddn Nicaragua

    The error that comes from this module is attached:

  • πŸ‡¨πŸ‡³China lawxen

    Same problem with #15 with

  • A commercial building cost estimator utilizes data analysis and industry expertise to project accurate construction expenses. By assessing factors like materials, labor, permits, and site preparation, these estimators provide vital insights for budget planning and project management. Advanced software and algorithms further enhance efficiency, ensuring precise forecasts for developers, contractors, and investors, ultimately facilitating informed decision-making in the construction process

  • A commercial building cost estimator utilizes data analysis and industry expertise to project accurate construction expenses. By assessing factors like materials, labor, permits, and site preparation, these estimators provide vital insights for budget planning and project management. Advanced software and algorithms further enhance efficiency, ensuring precise forecasts for developers, contractors, and investors, ultimately facilitating informed decision-making in the construction process

  • πŸ‡ΊπŸ‡ΈUnited States greenskin

    I like the idea of #5 but need it for setting a font color class but the "view" option doesn't appear to work for fontColor. We try to use Tailwind CSS for our themes and would love the option to apply these style options (font, size, color, etc) as classes.

Production build 0.71.5 2024