[upstream] [Style] Unable to apply styles to <ul>, <ol>, <table> etc. in CKEditor 5

Discussed this with @Reinmar from CKEditor 5!

This is one of their top priorities for this quarter. The fix should get merged in March, and will ship in their April 2–5 release. 👍

However, it's not a single fix: it cannot be fixed generically. It requires changes in many parts of CKEditor 5: for <ul> the list plugin will have to be modified, for <table> the table plugin, and so on. So it's not known yet for which tags it will be fixed and for which ones it won't be — but expect a big leap forward in any case.

@Reinmar just confirmed that fixing this is scheduled for Q1 2023 👍

Due to unforeseen challenges with the conversion of the CKEditor 5 codebase to TypeScript, this has been delayed.

Update directly from @Reinmar: expect the first PRs solving parts of this issue to land mid-May, and for most/all of this to be solved by Q2.

Wim Leers → credited ChrisSnyder → .

As I reported almost a year ago at https://github.com/ckeditor/ckeditor5/issues/11709, this also applies to <a>. Clarifying that here.

Closing 🐛 CKEditor 5 Text Styles can be applied to any element even when a tag is specified Closed: duplicate as a duplicate.

As a temporary work-around for <a> specifically, you could install @ChrisSnyder's https://www.drupal.org/project/ckeditor_link_styles → module.

Wim Leers → credited ericras → .

Wim Leers → credited jds1 → .

Crediting the contributors of that issue.

Also note: 📌 [Style] Warn the user about styles for unsupported elements Needs work would reduce the "WTF" factor — hoping to hear from the CKEditor team soon on that one! 🤞

Note for , it seems to work fine. My defined styles for them can be applied to links I added. The problem is that it allows applying those styles to any element instead of just . That wasn't super clear to me when reading thru this issue

Good point — clarified.

Thanks for keeping us up-to-date on this. I've hoped it was resolved by this month but I can see that it's not until mid May / Q2.

If not solved in time, can we use CKE4 with D10 ?

@brulain Yes, you can use CKEditor 4 with Drupal 10 for another ~6 months. See https://www.drupal.org/docs/core-modules-and-themes/deprecated-and-obsol... → .

<div> is also affected.

1. Drupal 9. Set CKEditor 4 in Full HTML text format.
2. Define the style div.alert.succcess|Success alert.
3. Click on “Success alert” in “Styles” dropdown on a node body field and it inserted <div class="alert success"> </div> and placed the cursor inside.
4. Switch input format to CKEditor 5.
5. Expand the “Styles” dropdown on a node body field, and the block style is disabled.
6. Explicitly allow the <div> tag in the manually editable tags setting for CKEditor5.
7. Manually add a <div> in Source mode on the node edit form body field.
8. Switch back to normal editing mode and ensure cursor is inside the DIV.
9. Expand the "Styles" dropdown and the block style is still disabled.

There is also an issue with heading tags. They are disabled. Here is a list of styles I currently have.

h2|Heading 2
h2.jump-link|Jump Link
h3|Heading 3
h4|Heading 4
p.p-lg|Large Paragraph Copy
p|Body Copy
small|Mouse Type
span.notranslate|No Translate
span.sl_swap|Content Swap

@jeremy.sloan:

That is an interesting point about the functional regression. Before you could actually create an "empty" style, and that would be enough to wrap the currently selected text in the block-level tag like div, h2, h3 etc.

the way it appears to work now is that you must first change the selected text from "Paragraph" to "Heading 2" (so that the H2 tag can be applied in ckeditor source mode) before the Heading 2 jump-link style can be applied. This workaround obviates the need for the naked H2 style in the styles list.

Wim Leers → credited Luke.Leber → .

Closed 🐛 [upstream] [Style] Support adding class attribute to table captions Closed: duplicate as a duplicate.

Quoting @Luke.Leber:

When configuring a style for caption elements, it cannot be applied to the table plugin model through the styles drop-down. Incidentally, it also cannot be applied through source editing mode, as it's stripped out, which might be a GHS issue.

See https://github.com/ckeditor/ckeditor5/issues/13777 for the blocking upstream issue.

Steps to reproduce

1. Add caption.test|Caption test as a style option.
2. Observe that the style cannot be applied via the styles dropdown.
3. Add <caption class="test">Bah humbug</caption> via source editing mode, switch out of source editing mode, and back in, observing that the class attribute is gone.

The good news: https://github.com/ckeditor/ckeditor5/issues/13777 was fixed on April 28! 😄

@jwilson3 ok that makes sense.

1) Put in you text first with headings plugin
2) Highlight text and apply styles. (Only styles that can be applied to the tag will be enabled)

Whats the timeline for rolling this into core? Will it be in the next version of 9 as well?

Is this really fixed by https://github.com/ckeditor/ckeditor5/issues/13777? It looks like that issue is only about table based elements.

Looks like we are still waiting on:
https://github.com/ckeditor/ckeditor5/issues/13341
https://github.com/ckeditor/ckeditor5/issues/11577

Both are needed to close this issue.

Updated IS

@frob: I posted an update in the #ckeditor5 Drupal Slack channel after the meeting I had last Thursday with the CKEditor team. Sorry for not cross-posting here!

Paraphrasing what I wrote there:

• Most fixes for Style will ship in the next CKEditor 5 release, which will happen with 95% certainty on May 22 (twelve days from today!). See https://github.com/ckeditor/ckeditor5/issues/11574 for the list of things that is fixed (progress is still being made).
• I've already got confirmation from Drupal core release manager @catch that we'll update 10.1.x despite it already being in alpha from the current 37.1.0 version to the next release.
• In other words: many Style problems will be fixed in Drupal 10.1.0!

Rather than manually keeping track of a list of upstream issues here, I'd rather point to a single umbrella/meta upstream issue (so updated the IS), and then after the next release they will create a new umbrella/meta issue for the remaining ones.

I agree the clearest way forward then is for us on the downstream Drupal side to keep this issue open until Style works on all commonly used HTML tags it's known to not work on.

Will it be in the next version of 9 as well?

Drupal 9 will not get any CKEditor 5 updates, because Drupal 9.5.x is the last minor version for Drupal 9.

Drupal 9 will not get any CKEditor 5 updates, because Drupal 9.5.x is the last minor version for Drupal 9.

I am not quite sure I get this. If the fixes come before EOL of Drupal 9 why wouldn't 9.5 get the fixes? I thought 9.5 was a part of the CKE4-5 upgrade path for D10

Version 38 has been released: 📌 Update CKEditor 5 to 38.0.0 Fixed .

@frob: because each CKEditor 5 update is a major release with some backwards compatibility breaks. That's why only new Drupal core minors get CKEditor 5 updates: there's too much disruption risk otherwise.

📌 Update CKEditor 5 to 38.0.0 Fixed is green: tests are passing. That means it's time for manual testing here! 🥳

We know for a fact that things are better in this release, so Drupal core will update to this version. But now we need to construct a list of the remaining bugs.

What remains

Currently there are 13 open issues for the Style package and 29 closed. Here are the 13 open issues:

In the stabilization issue, 14 of the 20 issues are completed:

Impact of what remains to be fixed

I think the most impactful remaining bugs are:

1. <div> support: https://github.com/ckeditor/ckeditor5/issues/13341
2. <table> support: https://github.com/ckeditor/ckeditor5/issues/11577

Impact of what has already been fixed

• <blockquote> support: https://github.com/ckeditor/ckeditor5/issues/11576
• <ul> + <ol> support: https://github.com/ckeditor/ckeditor5/issues/11590
• <a> support: https://github.com/ckeditor/ckeditor5/issues/11709
• … and many more: https://github.com/ckeditor/ckeditor5/issues/11574

Please confirm!

Please confirm the above breakdown in your manual tests 😊🙏

Still seems somewhat buggy.

I had 2 styles

ul.test|Test0
ol.test1|Test1

On a basic page
Added a ul list.
The style dropdown updates correctly.
I can add a style to the ul tag
GOOD

I press enter 3 times to get out of the list
Start a ol list.
It has the class for ul tag and I can't remove it
I can add ol style fine

So should that one issue be moved to a follow up?

#38: I think that might be the already known bug described in https://github.com/ckeditor/ckeditor5/issues/11606. Asking @Witek Socha for confirmation… 😊

Also don’t think this bug is unique to lists. Seen it with other tags. If you press enter it doesn’t clear any settings

Yep, that's exactly what #11606 upstream says. Makes it more likely that @Witek Socha will confirm my hunch.

I confirm this is the issue, I created a separate ticket for the list case https://github.com/ckeditor/ckeditor5/issues/14216. We will discuss it internally, and I will keep you posted.

@frob FYI I have done some work on back porting v38 to Drupal 9.5. I now understand why it is not being back ported past Drupal 10.1

I have done the back port and it is working well enough to test this issue and confirm that when we upgrade to 10.1 will be able to move to CkEditor 5. However in v36 up (which were both in D10.1) there was a change in the plugin API which caused crashes of CKEditor 5. (See https://www.drupal.org/project/editor_advanced_link/issues/3350254) which I have found it also effects link it as well.

For testing of v38 i think this is fine, but I do not think this should be used in production.

Think a follow-up can be opened for tracking bug in #38, since it's not unique to ol, ol or any tag but whenever styles is used.

Not sure if Drupal wants to add some test coverage for this too, even though it was fixed upstream.

@wim leers thoughts?

Tests

I personally don't think it makes sense to test every single one of these cases in Drupal if they're already being tested upstream. OTOH, it would make it crystal clear which precise cases are not yet working… 🤔

And in fact \Drupal\Tests\ckeditor5\FunctionalJavascript\StyleTest::testStyleFunctionality() already exists for that very purpose. It currently contains for example

    // @todo Uncomment this after https://github.com/ckeditor/ckeditor5/issues/11709 is fixed.
    // $this->assertSame('true', $buttons[3]->getAttribute('aria-disabled'));

So: self-assigning to update (and expand) that test coverage.

To follow up or not

IMHO we should not create a follow-up but instead keep this issue open. Because it already has ~46 followers; we shouldn't ask all of them to follow another issue? OTOH, that issue could be more narrowly scoped, and we'd stop notifying people whose problems are already solved.

Thoughts?

Remaining known bugs have a new upstream "meta" issue: https://github.com/ckeditor/ckeditor5/issues/14274.

That approach makes sense to me!

Just wanted to add a comment to say that the upstream epic that was being tracked appears to now be complete and was marked for release in v38.0.0 of CKEditor 5.

https://github.com/ckeditor/ckeditor5/issues/11574

#48: Indeed, see #37 through #46.

Thanks to @Spokje in 📌 Uncomment assertions in StyleTest related to https://github.com/ckeditor/ckeditor5/issues/11709 Fixed , part of #45 is already addressed now 👍

Met with the CKEditor 5 team. yesterday. I have GREAT news! 🥳

They made great progress on https://github.com/ckeditor/ckeditor5/issues/14274, with the first 3 being solved already:

1. [GHS/Style] List styles preserves itself and spreads to other list types and blocks. #14216
2. [Style dropdown] Inline styles retained when changing heading, inserting codeblock #11591
3. [Style dropdown][GHS] Clean up disallowed attributes upon editing #11606

👆 These will ship in a 38.x minor release, with no BC breaks, at the end of June! That will solve #38.

They are already working hard on "Can't set the style on

👆 These will ship in a 38.x minor release, with no BC breaks, at the end of June!

Most likely 🤞, we haven't yet pinned the version for the next release yet.

The fix for <div> support landed upstream 46 minutes ago: https://github.com/ckeditor/ckeditor5/commit/4b44f58c8dd280cafaadb10b2d0... 🥳

Did some testing and was able to apply custom style classes to <ul> and <ol> and <table>s without issue on the latest 10.1.x 🔥.... Unsure of <div>s though, don't have the plugin installed.

Think we should still include test cases so this functionality doesn't break on Drupal.

I have expanded the FunctionaJavascript/StyleTest.php coverage to test styles for ul, ol and table elements (https://git.drupalcode.org/project/drupal/-/merge_requests/4462/diffs?co...).

It's not possible to apply a style to a div element is not working yet as far as I know.

My first commit failed because it worked in my 10.1.x version but fails in the latest code, probably because of this ckeditor issue that has been fixed: https://github.com/ckeditor/ckeditor5/issues/11709

Committing an attempt to fix it, blindly because I don't know exactly how to update or rebuild my local ddev setup to test the latest dev branch changes (setting the ddev/ddev-selenium-standalone-chrome addon which needs core-dev). I keep trying to figure that out.

Updating to 10.1.x, that's why test was failing.

Tests passing, this is ready for review

The MR should actually point to 11.x as that’s the current development branch. Could that be updated please

Updated title and IS to what is being fixed.

Removed follow up tag as issue appeared to be fixed upstream.

The test coverage appears to be good adding coverage for ul, ol, and table. But yes don't believe there is a core button for adding a div. There is a contrib https://www.drupal.org/project/ckeditor_div_manager → but not ck5 version yet.

Think this is good to go.

Hi,

What about Drupal 9, the bug is still there for the div?

Thanks

#64: this will not be backported to Drupal 9. That would break contributed modules.

The test coverage that @juanolalla added looks great! 🤩👏

I've added test coverage for <a>. I'm working on test coverage for <div>. Will wrap that up in the next few days.

Ok fine, I will upgrade to 10 so, how can I apply the fix?

It’s already fixed 😊 This issue is only adding test coverage!

Ok because, I tried in Drupal 10.1.2 after having updated from 9.5 and when I save the styles in the ckeditor configuration, I got this error:

AssertionError: assert(NestedArray::keyExists($subform, $parts)) in assert() (line 892 of core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php).

I just tried to put that:
div.intertitre|intertitre

Thanks

Hi Again,

I make found a solution, when you put the styles you have to first click our the fiel to have the ajax call to save the styles only after you can save the form.

So now I have the style correclty setup, but the stlye button in CKEditor is still disabled, I have put those styles:
div.texte|Texte
div.intertitre|Intertitre
div.exerge|Exerge
div.encadrer-titre|Encadrer Titre
div.encadrer-texte|Encadrer Texte
div.itw-question|ITW Question
div.itw-reponse|ITW Réponse
div.note-bas|Note Bas

But the button is not working, do I need to do something ?

Thanks

#68 + #69: that's related to #3262484: AJAX desync when quickly changing multiple form values in plugin settings vertical tab → and #3265626: Changes to "Manually editable HTML tags" lost if form is submitted without triggering AJAX → . It's unrelated to this issue.

Added the missing test coverage.

This is now ready.

Since we don't have a button that makes div I did have to manually add to the source. But confirmed I could add a style class to the div.

Yep, and that's also what the test coverage does 👍😊

Thanks for the very fast review! 😳🥳

Just to know, are there any plans to return to having a behavior similar to ckeditor4 regarding styles, where it was not necessary to manually add the <div> declaration to the source, just select the text and the <div> comes automatically with the style?

Yes, we plan to have a similar behavior in CKE5. We are focusing on the most impacting bugs and I don't have an ETA yet.

Indeed. That's also what I explained in #51 ~2 months ago.

If there's at least one more person who posts a comment asking about this, I'll create an explicit issue on Drupal.org to track this upstream issue, that way Drupal users can get notified when Drupal updates to a version of CKEditor 5 that supports this 😊

Hi,

Finally I have move from div to p, it work well. Quick question, how can we disable the "multiple style" behavior? The user need every time to click to remove the previous one, it's not really convenient like that.

Thanks

Quick question, how can we disable the "multiple style" behavior? The user need every time to click to remove the previous one, it's not really convenient like that.

Please note, that others desperately need that functionality. So perhaps that should be an option then to opt-in or out? (As separate feature request / follow-up discussion?)

Agreed with @Anybody. That's how it also worked in CKEditor 4 (try it for yourself at https://ckeditor.com/ckeditor-4/demo/), it just didn't show "Multiple styles" as the label.

I checked the docs (and demo) at https://ckeditor.com/docs/ckeditor5/latest/features/style.html, and there's nothing about this sort of "mutually exclusive style". But … there is a feature request for it: https://github.com/ckeditor/ckeditor5/issues/14206 — please add a reaction to that — that's how the CKEditor 5 team prioritizes issues based on their userbase's input! 😊

thanks a lot

It looks like we are still missing test coverage for the bug reported in #29. I think it would be great if we could expand the coverage to confirm that it has been addressed too.

Added test coverage for #29 — good catch! 👍

Happy to report that this one too is working correctly 😄

Re-RTBC'ing because this is identical to the pattern established by all the other test cases — this is just one more test case.

Committed 2014af7 and pushed to 11.x. Since this is test only change, backported to 10.1.x. Thanks!

lauriii → closed merge request !4462

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

image styles are not working on 9.5.10 cKEv5
exp:
img.m3|Margin

div , a tags are picking up

@thatipudir This only landed in 10.1.x and newer. Please update. 🙏

One additional regression was reported: <br class>. See 🐛 [upstream] [Style] does not work Active . The test coverage there builds on the test coverage that was introduced here 👍

@wim Leers - Issue on both 9.5.10 and 10.1.x.

@thatipudir I think it is save to say that this will never work on Drupal 9.5

Quoting #51:

They're splitting that up in two pieces: the critical piece (being able to apply styles to <div>s) will happen first, the non-critical piece (being able to convert an existing element, for example a paragraph, into a <div class="something"> by just choosing a style).

There now is a Drupal.org issue to track this remaining upstream non-critical piece: #3362451-9: [upstream] [Style] Allow CKEditor 5 to *create* a

directly (instead of first

, then adding a class) → .

As I wrote already in the followup issue: https://www.drupal.org/project/drupal/issues/3362451 ✨ [upstream] [Style] Allow CKEditor 5 to *create* a directly (instead of first , then adding a class) Needs work

Please consider opening another issue for ck5 and div, as that problem also appears to be still unsolved, and it is slightly different than for "headline" because divs can wrap multiple elements. Alternatively change the title of the followup issue so that it does not only refer to headlines.

a.a-button|Button doesn't seem to work. Anchor tags don't work anymore?

Is there any projection to solve the image class problem in CK Editor 5?

It is still impossible to convert a <p> into a <div> automatically by selecting from the Styles dropdown?

And if yes, why are this issue and the upstream issue marked as closed ?