Cygnet styles leaking into CKEditor 5 editor

Created on 26 April 2023, about 1 year ago
Updated 19 September 2023, 9 months ago

Problem/Motivation

When using CKEditor 5 with the Cygnet "theme" (not a Drupal theme, but the JSON Editor theme packaged in Patternkit module), some styles from Cygnet are leaking into the CKEditor editor.

For example, a <p> tag in the editor gets a left margin of 20 pixels, due to this rule:

#drupal-off-canvas [data-theme=cygnet] p {
    margin-top: 0;
    margin-left: 20px;
}

Steps to reproduce

- Install Patternkit 1.0-beta7 on Drupal 9.5 or 10.
- Enable module Patternkit Example.
- At /admin/config/user-interface/patternkit/json:
- Set to use CKEditor 5 as the wysiwyg plugin.
- For field "CKEditor toolbar", select a text format that uses CKEditor 5.
- Select "Cygnet" for field "Select the JSON Editor Theme"
- On a Layout Builder page, add a block of type "Example with filtered content".
- In that block's wysiwyg-enabled field, add some <p> tags. Notice the left margin.

Proposed resolution

Drupal core already provides a system to adjust CSS rules to not target CKEditor 5 window (see ckeditor5SelectorProcessing in /core/modules/ckeditor5/js/ckeditor5.es6.js).

This approach (called called donut scoping) only works if the stylesheets have been loaded into the DOM. If our code dynamically adds a CSS file to the DOM, we need a way to ensure the file is fully ready before Drupal's donut-scoping processor runs.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Needs review

Version

9.1

Component

Schema Editor

Created by

πŸ‡ΊπŸ‡ΈUnited States krisahil

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

Comments & Activities

  • Issue created by @krisahil
  • πŸ‡ΊπŸ‡ΈUnited States krisahil
  • πŸ‡ΊπŸ‡ΈUnited States krisahil

    Turns out that Drupal core already provides a feature to add "donut scoping" to stylesheets, so that those stylesheets don't target CKEditor 5 instances (see offCanvasCss in ckeditor5.es6.js). However, this method is not consistently applying to stylesheets for Patternkit's JSON Editor (JE) themes (in a large Drupal 9 site, this feature is working, but in a small D9 site, it is not). I believe the problem is due to a race condition, where PK adds stylesheets to the DOM, but those stylesheets are not in the `document.styleSheets` list.

    I think we should re-consider how we add JE theme assets to the DOM. Instead of using JS to add the stylesheets, why not attach them as regular Drupal libraries?

    Here's a change that seems to work ok for the Cygnet JE theme, in my very limited testing:

    diff --git a/js/patternkit.jsoneditor.js b/js/patternkit.jsoneditor.js
    index 9bbbefc..42a2fb6 100644
    --- a/js/patternkit.jsoneditor.js
    +++ b/js/patternkit.jsoneditor.js
    @@ -1410,17 +1410,6 @@ function _createClass(Constructor, protoProps, staticProps) { if (protoProps) _d
     
             var editor_dom = '';
     
    -        if (this.settings.themeStylesheet) {
    -          var theme_element = document.createElement('link');
    -          theme_element.rel = "stylesheet";
    -          theme_element.id = "theme_stylesheet";
    -          theme_element.href = this.settings.themeStylesheet;
    -          document.getElementsByTagName('head')[0].appendChild(theme_element);
    -          editor_dom = '<link rel="stylesheet" id="theme_stylesheet" href="' + this.settings.themeStylesheet + '">';
    -        } // @todo Re-eval with this shadow dom webfont bug:
    -        // https://bugs.chromium.org/p/chromium/issues/detail?id=336876
    -
    -
             if (this.settings.iconStylesheet) {
               var icons_element = document.createElement('link');
               icons_element.rel = "stylesheet";
    diff --git a/src/JSONSchemaEditorTrait.php b/src/JSONSchemaEditorTrait.php
    index ac2e45d..512d29e 100644
    --- a/src/JSONSchemaEditorTrait.php
    +++ b/src/JSONSchemaEditorTrait.php
    @@ -114,15 +114,6 @@ trait JSONSchemaEditorTrait {
     
         if (isset(PatternLibraryJSONForm::THEMES[$theme])) {
           $theme_info = PatternLibraryJSONForm::THEMES[$theme];
    -      if (!$editor_settings['useShadowDom'] && !empty($theme_info['css_no_shadow_dom'])) {
    -        $editor_settings['themeStylesheet'] = $this->getLibraryAssetUrlFromUri($theme_info['css_no_shadow_dom']);
    -      }
    -      elseif (!empty($theme_info['css'])) {
    -        $editor_settings['themeStylesheet'] = $this->getLibraryAssetUrlFromUri($theme_info['css']);
    -      }
    -      else {
    -        $editor_settings['themeStylesheet'] = '';
    -      }
           if (!empty($theme_info['js'])) {
             $editor_settings['themeJS'][] = $this->getLibraryAssetUrlFromUri($theme_info['js']);
           }
    @@ -196,6 +187,13 @@ trait JSONSchemaEditorTrait {
           ],
         ];
     
    +    if (!$editor_settings['useShadowDom'] && !empty($theme_info['css_no_shadow_dom'])) {
    +      $element['#attached']['library'][] = 'patternkit/cygnet_no_shadow_dom';
    +    }
    +    elseif (!empty($theme_info['css'])) {
    +      $element['#attached']['library'][] = 'patternkit/cygnet';
    +    }
    +
         if (!empty($editor)) {
           $element['#attached']['library'][] = 'editor/drupal.editor';
    
  • πŸ‡ΊπŸ‡ΈUnited States krisahil

    An alternative approach is to delay loading CKEditor until after the JSON Editor (JE) theme assets have been loaded. Here's a patch that tries to do this. This would be a quicker fix than loading the JE theme assets via Drupal's library system.

  • πŸ‡ΊπŸ‡ΈUnited States krisahil
  • πŸ‡ΊπŸ‡ΈUnited States krisahil
  • Status changed to Needs review about 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    326 pass
  • πŸ‡ΊπŸ‡ΈUnited States krisahil
  • πŸ‡ΊπŸ‡ΈUnited States slucero Arkansas

    To fix this, maybe we could load the JE theme assets via Drupal's library system, instead of manually injecting them by JS.

    The change you mentioned here has been documented as a follow-up improvement to address via πŸ“Œ Attach JSON Editor Theme Assets Using Library Attachments Postponed . I'm relating the tickets now to document that better.

Production build 0.69.0 2024