Problem/Motivation

Since #3201641: Improve the HTML filter configuration UX the fact that CKEditor 5's <img> functionality does not support width and height attributes. Which also means that they will not end up in the "allowed HTML tags" setting for the filter_html filter 🙈 This massively complicates the update path.

It's not supported natively — see https://github.com/ckeditor/ckeditor5/issues/5154

Maybe a work-around is possible:

Resizing images is documented here: https://ckeditor.com/docs/ckeditor5/latest/features/image.html#resizing-.... At least with CKE5 28.0.0 this doesn’t work for us because the width is saved on the figure element wrapping img.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork ckeditor5-3222847

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Wim Leers created an issue. See original summary.

Wim Leers credited lauriii.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

lauriii opened merge request !72

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
  1. The image resize requires drag and drop which is an accessibility issue. AFAIK our test tooling also don’t support drag and drop which makes testing this harder. CKE provides alternative method to resize images using buttons to choose from preset values. Any thoughts on using that https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/features/image...?
  2. The MR provides support for width attribute since that was simple to map to CKEditor 5 model. What do we want to do with the height attribute? Do we want to support scaling images to disproportion by allowing resizing the height?
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

We should not require resizing to get width and height set. 😞 Let's discuss this with the CKEditor team.

  1. Hm … this would be a superior UI to actually finally do #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5 but that is A) a net new feature, B) technically out of scope, C) still not solving the original problem. I am not opposed to it, although I do worry it'll lead to new problems/challenges… so I wonder if we should just open a follow-up for that?
  2. I'd say we should just never allow disproportionate resizing. Contrib can add that edge case capability if it wants to.
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs work

Outcome of meeting with @Reinmar:

  1. It's much more complicated to support this in CKE5 upstream because of cross-compatibility issues than it is for us to explicitly add/generate width and height.
  2. So at least for now Drupal should implement/own this.
  3. In the current MR there's a bug: it generates width="100px" instead of width="100" 🤓
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

@lauriii pointed out in chat that width and height are not generated in CKE4 either. So … what's up with that?

Turns out that we rely on \Drupal\editor\Plugin\Filter\EditorFileReference to generate it! I forgot about that! 😅

Fortunately thanks to the existing infrastructure it's a one line addition to guarantee the presence of that filter:

diff --git a/src/Plugin/CKEditor5Plugin/ImageUpload.php b/src/Plugin/CKEditor5Plugin/ImageUpload.php
index 24771b9..d2e29d5 100644
--- a/src/Plugin/CKEditor5Plugin/ImageUpload.php
+++ b/src/Plugin/CKEditor5Plugin/ImageUpload.php
@@ -31,6 +31,7 @@ use Drupal\editor\Entity\Editor;
  *   conditions = {
  *     "toolbarItem" = "uploadImage",
  *     "imageUploadStatus" = true,
+ *     "filter" = "editor_file_reference",
  *   }
  * )
  */
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Related issues: +#3224652: [drupalImage] Add ckeditor5-image's imageresize plugin to allow image resizing

Working on MVP as part of this problem which is to ensure that width and height are retained when configured, as well as ensuring that when the attributes don't exist, they are generated by the file reference filter.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

I'm wondering if #8 leads to potentially more confusing UX than we've had previously. AFAIK, the toolbar item conditions are not displayed in the UI so using the filter condition could be confusing, especially in this case given that it is not a hard requirement. I'm wondering if we could move #8 to a follow-up where we improve the UI so that it either automatically enables the filter or provides guidance on how to make the toolbar item visible in the UI.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community

You currently also do not get guidance that you need to enable Enable image uploads. But at least that is visible.

So I 100% agree with #10.

Also, in manual testing, I could not fault this at all …

  • Wim Leers committed a13b29e on 1.0.x authored by lauriii 
    Issue #3222847 by lauriii, Wim Leers: <img width height>
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Needs followup
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Follow-ups:

  1. editor_file_reference filter must be enabled
  2. image resizing functionality for CKE4 functional equivalence
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs followup

Follow-ups filed:

  1. #3225033: Whenever "Enable image uploads" is enabled for a text editor, the editor_file_reference filter should be enabled
  2. #3224652: [drupalImage] Add ckeditor5-image's imageresize plugin to allow image resizing

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.