CKEditor 5 isn't respecting field widgets row settings

CKEditor 4 does not actually respect this either… see #2877527: Make CKEditor (approximately) reflect the rows setting of a field.

Related upstream issue: https://github.com/ckeditor/ckeditor5/issues/636

am i reading it correctly that his recommended approach would be to set the height of the editor window by css?

That is my interpretation too.

then the upstream label might be not necessary anymore? would it make sense to implement that directly in drupal?

Well … no … how do we convert rows="N" which essentially means "N lines of text" to a concrete height?

The point is that we shouldn't be implementing this here in Drupal, it should be implemented upstream.

There just is no reliable way to implement this, which is also why CKEditor is trying to not implement it upstream.

If you start to enter content now the summary field behaves as expected with CKEditor 4 & 5 - only 3 rows are visible and as soon as more than 3 lines are entered a scrollbar is shown. For CKEditor 5 it behaves unexpected again.

The summary field does not use CKEditor 4 or 5, it's just a plain <textarea>. This is why it "works": because it has a predictable line height.

Sorry to not have better news 😔

Reported again at #3273744: CKE5 Respect a MINIMUM size for the CK-enabled textarea:

At the moment, CKE5 textarea do not respect any configurable sizes, although as a site builder I am provided with a UI to configure so.

[…]

Using the standard install profile as an example here, you are provided a UI to configure a number of rows for your textarea. Example is shown here with the default body field of the default article node type available at /admin/structure/types/manage/article/form-display

Enter 9 here for instance : on CKE4 this is not respected, but the default size is hardcoded (within CKEditor I suppose) to 5 :

After conversion to CKE5, this very same field as a size of 1 which make it weird to people used to Drupal.

All I can do here is say: please read https://github.com/ckeditor/ckeditor5/issues/636 and articulate how you believe this could be addressed upstream.

@rkoller in #8:

out of curiosity i've created a text snippet with 50 numbered lines and then copied and pasted that. ended up with a 1400 line text field stretching the actual length of the page into "infinity" (mind the the scroller and it position in context to the height of the whole scrollbar) ;)

imagine the whole field filled with actual complex content and having 3 or 4 more ckeditor 5 fields plus several other elements in a drupal content type with limitless expanding ckeditor fields i consider the navigation between the different fields or the page in general might become difficult and challenging. and from a performance perspective does it make a difference if you have lets say three ckeditor5 fields that would be limited in height with only a section of each fields content visible in contrast to the current state where you have expanded ckeditor5 fields to the entire length of its content?

For that, @Dom. opened #3273755: CKEditor 5 should not grow to infinite height actually — let's continue that aspect in there? 🤞

To me this is a pretty big usability issue.

I talked to @lauriii and floated this idea. He seemed fairly receptive, so re-opening.

1. Expose the row settings to the JS if it's not already
2. Calculate the height of the CKEditor (something like row count * REM height * line-height)
3. Set that value to a custom property within CKEditor
4. Use that custom property to set a min-height within the stylesheet.

@mherchel That makes me curious why this was not a big usability issue in CKE4 for you? Same challenge there: #2877527: Make CKEditor (approximately) reflect the rows setting of a field.

Good question.

The reason is because (at least for the default body field), the field size is tall in D9. IMO this gives the content editor a sense of the purpose of the field (the body field is meant for long form content).

While in Drupal 10, the content editor does not get this sense of purpose.

Does this make sense?

Ah, yes, that does make sense!

Implements approximate number of rows spacing in the Ckeditor5 initial load.
The editor area still resizes on click in side it to editors default behavior.

As inspiration served this module https://www.drupal.org/project/ckeditorheight for Ckeditor4

Very interesting, @Emil Stoianov! And since that has a fairly solid amount of usage, we may actually consider that in Drupal core… hoping @nod_ or @lauriii will review this 🤞

Moving to NW for all the tags needed. Also patch #19 failed.

+++ b/core/modules/ckeditor5/js/ckeditor5.js
@@ -218,8 +218,16 @@ function _arrayLikeToArray(arr, len) { if (len == null || len > arr.length) len
+        var toPx = function() {
+          var element = jQuery('<div style="display: none; font-size: 1.5em; margin: 0; padding:0; height: auto; line-height: 1; border:0;">&nbsp;</div>').appendTo('body');
+          var height = element.height();
+          element.remove();
+          return height;
+        };

Could we get the height of the ckeditor text area instead of a whole new thing? Since the textarea only show one line we can get the are height "easily" maybe? That would account for styling differences between wysiwyg areas in case there is one

Patch Doesn't apply to 10.1.x

I think the simplest (but not most flexible) way would be to set a min-height to .ck-editor in the admin theme like this;

.ck-content{
  min-height: 300px;
}

So this way the editor is in a minimum height on initial and it will expand to the bottom if there is more content entered.

Edit: This is only a quick fix for people with urgent needs :)

Rerolled for 10.1.x.
Please review.

@WimLeers said in #20:
> Very interesting! And since [ckeditotheight.module] has a fairly solid amount of usage, we may actually consider that in Drupal core… hoping @nod_ or @lauriii will review this.

Although being late to the party, as the author of ckeditorheight i can give some 5 cent.

In all that discussion, we had that point that "there is no undisputable way to convert rows into heught".
The nerd in me said yes, the practician smiled and said nope.
So created ckeditorheight, did not look for two years, and now i'm impressed by the usage, too.
Looks like the approach works for some.

So what does it do? It calculates the height like this:

$height = $offset + $lineHeight * $rows;

Also the $unit (pt, em, rem) is configurable, and "disable autogrow".

That's it, no rocket science, just linear algebra. Any ckeditor5-Api versatile person should be able to do that in half a day. (I'm not and the decorator voodoo i did in ckeditorheight (sorry kittens) likely originated fom my lack of Api-grokking.)

I can't put free time into this atm. But:
- I'd love to see this in core
- I postponed and pointed the ckeditorheight issue to this one
- IF core maintainers feel like this should be pioneered in contrib, feel free to create a MR in the related issue.

Just a sysadmin here, but please also take a look at https://www.drupal.org/project/ckeditor/issues/2877527#comment-14810796 which might be of interest implementation wise.

Regarding the use cases:
I have another one here. I don't want the editor to have a given minimum height and expect it to grow, quite the opposite, I want one single, fixed line.
Might sound strange, eh? I need it for an a11y hack as Drupal has no way of marking up different languages in the title field of content (as it does not allow HTML; which should be tackled elsewhere). We fell through an a11y audit because of this. Now we have an additional title_html field that has just one line (realized with ckeditorheight, thank you for developing it!) which overrides the actual title field if filled.

Regarding the patch:
I don't know if jQuery will be included/be part of CKEditor forever. Maybe element should be determined with vanilla ES6?

@tgoeg have you seen the Language of Parts under CKEditor?

I've done lots with multi-lingual sites. Why would you want to have a title different than the body of the content? How are you adding languages using a title with your work-around?

Seems that wasn't clear enough, sorry.
We need to mark up multiple mixed languages in one title.
I am from Austria and there are many terms we tend to borrow from English, "Download" for example.
If we do not add correct markup for screen readers to understand this is an English term, it will be read out as something like "Dovnloh-ahd', which might be funny but not that helpful.
You need to correctly mark parts in other languages to get web accessibility certifications, which is easy with CKEditor's language button (which just generates a <span lang="en">Download</span> for example).
As this is HTML, I cannot put this into Drupal's default title field, however.
Title fields that embrace several rows are not only unexpected or distracting, they also promote entering the whole body right into this rich text field.
And that's where we're back on topic, sorry for the rather far-reaching excursion :-)

Now I am a unilingual person, but have been building multi-lingual websites in Drupal for over a decade. Mostly also keeping in mind accessibility concerns.

So rather than using something 'herunterladen' (I'm pretty sure I'm messing this up), people want to use the term 'download'

So, using Google Translate authors want to say something like "Download our current annual report", but rather than just using 100% German, 'Laden Sie unseren aktuellen Jahresbericht herunter', they want to use something more "modern" which sadly (my bias) has English sprinkled in, like 'Download Sie unseren aktuellen Jahresbericht'.

But because the page is in German, the screen reader user comes across the word "Download" the screen reader's library tries to read this in German and so pronounces it as you would in German "Dovnloh-ahd", vs how German people generally pronounce this English word, which would obviously be more like the English "Download"

So, you want a title the word "Download" isolated in English, so it is more like <h1><span lang="en">Download</span> Sie unseren aktuellen Jahresberich<h1>, so that it reads correctly?

If this is the case, interesting... Might be best to have a custom module that allows a user to include HTML in the text field. I think in most cases the assumption is that the title is just plain text.

Right, that's it.
But I don't think that's a German issue at all. English also has this requirement, with our national accessibility certification authority at least.
You also have to designate french words in English, think 'hors d'œvre' for example or the masses of other loan words that exist in English as well (be they French for BE or Spanish for AE for example)

The general assumption of a title field being plain text is ... plain wrong :-)
This should go into Drupal core, and not into a module, I think, as I should be able to create accessible sites ootb.

There are words that don't even have any translation. "Download" probably wasn't that good an example, maybe - in German at least - Smartphone, Computer, etc. The corresponding translations would be something like "intelligent telephone" or "calculator", which no-one would understand.

But we're getting off topic.

Entering a row amount for a CKEditor field should be respected, be it for a bigger or a smaller (even 1-row) setting.

We ran into this issue for Drupal 9.5 when we migrated to ckeditor5. Patch 3241295-row-weights-19.patch does not work with Drupal 9.5. Here is an updated patch along with a maxHeight as well.

After some more playing around I think setting minHeight to 100vh is the most useful. This way no matter what size device/window you are on/using you can benefit.

After more tests we discovered the style applied via JS is removed once the textarea is focused. We think the better approach is pure css. Latest patch attached.

Thanks for the patches @glynster

I first tried the CSS solution, unfortunately it did not have the desired success.

So afterwards I tried the Js solution, unfortunately it only works to some extent. In my case I use paragraphs with a text field, so in a node a new text paragraph is inserted, the edit field is clearly larger, whereby it does not correspond to the actual value of 15 rows. As soon as the mouse curser is placed in this field, the field is automatically reduced to 1 row.

This also happens with existing text paragraphs that already contain content. A maximum height is triggered, when the mouse is clicked the height is reduced to the amount of text.

@glynster: I don't see how #33 has variable height, but #32 has a good direction imho.
We can bikeshed if that kind of autodetecting line height can work, but that's detail (we can still make configurable like ckeditorheight.module, with fallback to autodetect).
This should use CKEditor API though (know, i don't know the api but asking on StackOverflow or CKEditor issue queue might help).
And of course... https://youmightnotneedjquery.com/

Attached is a working patch that solves the minimum height (setting the value to similar to the method in #31), and also setts the max height using both the viewport height and the new --drupal-displace-offset-top CSS variable that takes into account the toolbar height.

Note that I haven't extensively tested this.

@mherchel is it possible that you also create a patch for drupal 9.5? then i could test it directly for the drupal version.

@geek-merlin I guess in my eye using vh with css is a variable height based on the users display window. In our usercase it becomes super userful. Min height is not really a concern as apposed to the max height issue. Having to scroll up to the top of the textarea to get to the editor becomes a pain to work with over time.

@mherchel thanks for your patch however the issue remains that once the textarea is focused the listener removes all styles.

I wonder if it makes more sense to allow the option for Balloon, Balloon Block or inline. In our usercases it is about getting to the editor without the need to scroll to the top of the page every time. When you deal with very long articles this gets tiresome. CKeditor4 took care of this without you even knowing it.

https://ckeditor.com/ckeditor-5/demo/#balloon

Perhaps @mherchel can keep reworking his patch to accomodate a js listener so the inline style remains? Then we could have a win win? I would also love to get a working patch for Drupal 9.5.

@mherchel thanks for your patch however the issue remains that once the textarea is focused the listener removes all styles.

I work around this by adding a custom property to the parent .ck-editor element (instead of the .ck-editor__editable element), and then invoke that through the actual stylesheet.

Test it out, it seems to work well for me.

@mherchel you were completely right. I was manually testing this on Drupal 9.5 and forget to add the css. Totally stupid of me. Here is @mherchel patch for Drupal 9.5. It works well for us.

Verified and tested patch #33 on the drupal 9.5.x-dev version and the patch applied successfully.

Fixed CCF for #41 for 9.5.x.
Please review.

@_utsavsharma I wondered about es6! Thanks for the update. Still works well. Please test everyone and report back.

I verified the approach in #36 with CKEditor5 maintainer @lauriii.

A couple notes.

• Keep the max-height issue separate
• I'm assuming we'll need to test this somehow.

Attached is an updated patch for #36. This fixes some code formatting, removes the max-height, and adds very generous comments.

Still needs tests

@mherchel can you add in the es6 js file as well? Then the custom commands will pass like @_utsavsharma did.

Drupal 10 doesn't use the es6 files anymore (only D9 does). See https://www.drupal.org/about/core/blog/javascript-build-process-removed

It's failing because I wasn't using template literals.

If we do a Drupal 9 patch, we'll. need it.

Anyway, new patch incoming (with tests!!!!)...

Drupal 10 patch attached with tests!!! 😱

Test should pass on above, but I found a couple more (easily fixed) issues while manually testing:

• When switching to "view source", the rows height is not respected
• When adding a caption to an image, the figcaption also has a .ck-editor__editable CSS class, which gets the full height. We need to exclude this from the CSS selector.

Fixed the issues in found in #51.

Lets see what the tests say with this.

Interdiff for above.

🤦‍♂️ I included the interdiff in the patch above.

Either way, in this patch I cleaned up the tests a bit to make them more robust, including checking the height of the source code editor.

Cspell error above. This new patch modifies the dictionary.txt to add "sourceediting"

Nice!
Reviewing #55:
- Code looks solid
- Contains a nightwatch test
- Test-only patch fails that test (see console)
- Patch-plus-test is green

So according to the test, patch fixes the issue.

What's missing to RTBC is
- someone test this manually and provide screenshots for the maintainers to review
- someone text a change record

NW for that, after that imho RTBC.

Thanks for the review @geek-merlin!

Setting back to Needs Review per https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... because the patch [currently] does not need work.

I'm also setting this to a Bug report, because this is very much a bug IMHO.

We still need manual testing (with screenshots). Adding the Needs manual testing tag.

Editing issue summary to summarize results and add testing instructions. I'm narrowing the scope of this to only fixing the initial height of the editor. The maximum height issues can be handled separately in #3273755: CKEditor 5 should not grow to infinite height and #3326193: CKEditor 5 can grow past the viewport when there is a lot of content

Re: 62 I think it's important to distinguish between a line break and a paragraph. Using shift+enter should get you a line break (<br>) which should be equivalent in height to letting the text wrap on to the next line. A new paragraph in plain text would be equivalent to two line breaks.

Since the cke default would usually be a paragraph tag I think it makes sense to measure the line-height of a <p> tag, multiply it by number of rows and apply the top margin and bottom margin.

Note that in the case of a longer paragraph with no line breaks including the bottom margin might give you a little bit extra height to play with, but at that point I guess cke would grow to accommodate the bottom margin.

Since the cke default would usually be a paragraph tag I think it makes sense to measure the line-height of a

tag, multiply it by number of rows and apply the top margin and bottom margin.

Yeah, I'm coming around to that same opinion. Thanks for the input.

New patch attached that does all of this (please test).

Instead of injecting a <div>, it injects a <p> tag and then calculates its height and margins (accounting for collapsing margins), and outputs the correct height. It seems to work great for me.

Please test away :D

Adding in the radix parameter into parseInt. No changes to the test, so not re-uploading that by itself.

Tested and applied patch #65 on Drupal 10.1.x-dev.
Patch applied cleanly.
Issues got fixed after applying the patch

Refer to the attached screenshots
Before patch:

After patch:

RTBC +1

Just tested #65 locally and it works perfect. Thanks!

Since the scope now is to set the min-height only, I think this is fine to commit now. Since the patch is not introducing any regression (max-height is not changed in this issue) and it actually improving the UX setting the min-height, I'll move this to RTBC.

1. +++ b/core/modules/ckeditor5/js/ckeditor5.js
   @@ -370,9 +370,55 @@
   +           * Injects a temporary <p> into CKeditor and then calculates the entire
   

   s/CKeditor/CKEditor/

2. +++ b/core/modules/ckeditor5/js/ckeditor5.js
   @@ -370,9 +370,55 @@
   +            element.setAttribute('style', 'visibility: hidden;');
   

   Per https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#accessibilit..., this should be fine wrt accessibility.

   When I tagged this Needs accessibility review in #20 ~1.5 month ago, the solution was very different; now it's using CKEditor 5's built-in CSS properties 👍

3. +++ b/core/modules/ckeditor5/js/ckeditor5.js
   @@ -370,9 +370,55 @@
   +          // user inputted value of number of rows. We attach this custom property
   

   "user inputted value" sounds a bit odd 🤓

4. +++ b/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js
   @@ -0,0 +1,172 @@
   +module.exports = {
   +  '@tags': ['core', 'ckeditor5'],
   

   😳 Holy cow, this test coverage is something else! 👏

5. +++ b/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js
   @@ -0,0 +1,172 @@
   +        .pause(500) // Wait for machine name to update.
   

   I'm afraid this is pretty much guaranteed to introduce random failures 😬

   We should wait for an explicit element to appear.

6. +++ b/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js
   @@ -0,0 +1,172 @@
   +        ) // Wait for Ck5 settings to be visible.
   +        .click('.ckeditor5-toolbar-button-sourceEditing') // Select the Source Editing button.
   

   🐛 Multiple comment formatting problems, plus "Ck5" should be "CKEditor 5".

7. +++ b/core/modules/ckeditor5/tests/src/Nightwatch/Tests/ckEditor5EditorHeightTest.js
   @@ -0,0 +1,172 @@
   +        .keys(browser.Keys.DOWN) // Hit the down arrow key to move it to the toolbar.
   +        .waitForElementVisible(
   +          '[href*=edit-editor-settings-plugins-ckeditor5-sourceediting]',
   +        ) // Wait for new source editing vertical tab to be present before continuing.
   

   These and a bunch of the other comments use very creative formatting that is implicitly revealing that our JS linting is failing to catch these? 🫣

These and a bunch of the other comments use very creative formatting that is implicitly revealing that our JS linting is failing to catch these? 🫣

Yeah you can blame prettier for that! 🤣

Anyway, I moved around the comments and addressed all of the feedback above. Thank you for the review!

We're getting close!

I reviewed the code and can confirm that all the points from #68 have been addressed
... except some comment formatting in the JS test.

Still needs change record.

If we have both, i deem this RTBC.

Which comment within the JS test? I'm pretty sure I addressed them all...

I understood that in our standards the comments must be on their own line, whether php or js.
I may be wrong, but if i got that correctly, then comments like this are wrong:

+        .submitForm('input[type="submit"]') // Submit module form.

yeah, it's a little confusing IMHO.

Within https://www.drupal.org/docs/develop/standards/javascript/javascript-codi..., it states

If each line of a list needs a separate comment, comments MAY be placed on the same line and MAY be formatted to a uniform indent for readability.

I personally think this falls under that.

@ckrina found another instance where I had pause(500) within my test.

New patch attached.

All concerns from #68 have been addressed and @geek-merlin's concerns have been answered too. Plus the extra pause(500) from #74 too, so marking this as RTBC.

Assigning issue credits.

I'm wondering if this a bug or feature because it doesn't look like CKEditor 4 isn't taking into account this configuration. Also, the UI explicitly states that "Text editors (like CKEditor) may override this setting". Should we adjust that to remove specific mention of CKEditor since after this issue CKEditor 5 does take that configuration into account. 🤔

The issue where the text referenced in #77 was added #2717599: Text editors do not respect <textarea rows>: make that clear in the formatted text field widgets.

Text updated to say "Text editors may override this setting."

Based on RTBC in #75 and a grueling review of the interdiff in #79, back to RTBC.

Change record created at https://www.drupal.org/node/3330010

Code review

#72: That was also my understanding indeed.

But! core/tests/Drupal/Nightwatch/Tests/ajaxExecutionOrderTest.js and core/tests/Drupal/Nightwatch/Tests/claroAutocompleteTest.js apparently don't do this either. Perhaps it's a Nightwatch convention? 🤔 #73 answers that 👍

Change record review

I see two problems with the current change record though:

1. it does not distinguish between CKEditor 4 and 5
2. it claims With this change, Drupal will now show the proper amount of rows. → this is false, it's an estimation, and that's exactly why #2717599: Text editors do not respect <textarea rows>: make that clear in the formatted text field widgets modified the UI text. Note that rows is really a monospaced <textarea> concept, and only in that context can it work precisely. In the case of CKEditor 5, the line height of each line can be different — for example a subheading versus a paragraph, etc. The approximation algorithm Drupal applies to CKEditor 5 to make the rows setting apply approximately relies on a paragraph's line height.

Approach review

I'm in favor of this change after I first wanted to see CKEditor 4 support this upstream in #2717599: Text editors do not respect <textarea rows>: make that clear in the formatted text field widgets, which obviously never happened. Since #2877527: Make CKEditor (approximately) reflect the rows setting of a field has amassed a pretty high number of followers, and I've often been asked this question in person at Drupal events, I think this is a solid compromise 👍

Thanks for making this happen, @mherchel! RTBC +1

Screenshots — "typical before vs after"

#66 posted screenshots for a non-default rows setting. The default for text fields' widgets is rows=5, which looks like this

Before

After

Also added to the change record, since this will visually affect every Drupal site that is already using CKEditor 5.

Fixing a couple nits within the comments. Leaving to RTBC since only changing comments.

Opened a follow-up to try to improve the wording of the description: #3330159: Potentially confusing wording in text area widget.

Committed eddae13 and pushed to 10.1.x. Thanks!

should check that there is a row attribute with a value that is an actual number too.

I did check https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea#attr-... to ensure that rows attribute only allows numbers. I also checked that the rows attribute should exist always, see \Drupal\Core\Field\Plugin\Field\FieldWidget\StringTextareaWidget::formElement and config schema for field.widget.settings.text_textarea. Maybe we could do that as hardening in a follow-up?

🎉 Thanks everyone for driving this home! So the ckeditorheight prorotype will stay for CKEditor4, but finally this is in core for CKEditor5.

Happy New Year everyone. Thanks for working this out. Could someone please create a 9.5 patch? I've tried to hack it together but failed. Applying the #44 doesn't show a minimum height that more or less matches the number of rows on my body field.

I'm trying to update to 9.5 and implement all the changes before upgrading to 10.x (preferably 10.1.0 or later).

Sure thing! 9.5.x patch attached. Note this is not compatible with IE11.

Great! I can confirm the patch installs correctly and displays a minimum height of the textarea consistent with the number of rows. As a test I changed the number of rows from 8 to 5 and the height of the editor became smaller.
So here is at least one proof that the 9.5 patch works. Thanks a lot.

@hepabolu the bonus with #44 is the max-height. We find the max-height incredible useful whe you work with long form content. I am not sure where the seperate issue is, anyone know?

I'm in the middle of a big update so can't test this at the moment, maybe this use case outlined in #26 is now covered anyway.
But if max-height got removed as you imply then I think the row height setting is just a starting value and it still can grow further.

I don't think that's the primary functionality people need or the one expected if a row height can be set.
The auto-growing functionality is there, ootb, anyway. Don't people want to set this property when they need exact control of how big an editor field is, i.e. shorter because of the amount of editor boxes expected or even restricted to one line only?
Users can still manually expand the area if that's really needed (I guess?).

If really both is needed then maybe an additional checkbox "fixed height" would be useful?

> We find the max-height incredible useful whe you work with long form content. I am not sure where the seperate issue is, anyone know?
Try this:
#3273755: CKEditor 5 should not grow to infinite height

Help apprediated in driving that home too!

@geek-merlin awesome thanks for pointing me in the right direction. Will jump over there too!

@mherchel: since you're now the expert on this aspect of CKEditor 5, would you mind helping get #3273755: CKEditor 5 should not grow to infinite height across the finish line too? 🙏😊

This is a bug that affects also Drupal 9.5 and 9.5 is still long time supported (until December 2023). Not porting this to 9.5 is affecting the CKEditor 5 adoption. Most of the sites are now on Drupal 9.5 (or even 9.4), preparing for D10. Adopting CKEditor 5 is one of steps they want (true, not mandatory, but highly recommended) to solve before migrating to Drupal 10. But this is the kind of bug that wouldn't make CKEditor 5 module production ready.

Let's backport this, at least, to Drupal 9.5 to ease the CKE5 adoption. Luckily, @mherchel has already provided a patch in #91.

Let's try.

Try to fix Nightwatch test.

OMG yes, and it hasn’t even been committed to 10.0.x either! 😳

Queued a 10.0.x test of #83.

Not sure why none of us thought of this 😅 I actually think that's because the "row" setting also never worked for CKEditor 4, so this is technically a feature … but for CKEditor 5, this helps overcome the "tiny initial size" problem, so in that sense it's a bug report.

So … let's see what core committers think 😊

(Also: no idea why I tagged it as a string change 🙈)

So, while i agree that this should be going in 9.5/10.0, when I talked to @lauriii, his thoughts where this is a feature request (as opposed to bug). The reasoning was that the previous help text exempted CKEditor from having this behavior.

@mherchel, I've backported in #101 but there's a Nightwatch failure that needs some attention. Sorry, but I've never worked w/ Nightwatch

@mherchel thanks for confirming what I guessed in #102.

Up to committer's discretion then.

And actually, there is a string change:

+++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWidget.php
@@ -25,7 +25,7 @@ class TextareaWidget extends StringTextareaWidget {
-    $element['rows']['#description'] = $this->t('Text editors (like CKEditor) may override this setting.');
+    $element['rows']['#description'] = $this->t('Text editors may override this setting.');

This was just omitted from @mherchel's 9.5 backport in #91, which led me to the wrong conclusion. And that is actually a reason that makes backporting this even less likely… 😅

@claudiucristea I applied patch #101 with Drupal 9.5.7, ckeditor5 and it worked fine. Only there is one confusion, after applying this patch if we have set row as 5 in manage form display for the field then it is only showing as 2 on editor.
If we set 6 it is showing 2 rows.
If we set 7 it is showing 3 rows.
If we set 8 it is showing 3 rows.
If we set 9 it is showing 4 rows.
If we set 10 it is showing 4 rows.
and so on....

So is there any reason behind this or this is a bug?

Here's the patch which I rolled for 10.0.9 for a project

We're approaching the 10.1.0 release candidate and there's a fair amount of logic here plus a string change, so I think we should leave this fixed in 10.1.0. It is useful to have the backport patch from #108 for people that want to run it on 10.0.x

I can confirm that the patch at #101 is working correctly for me on Drupal 9.5.10, without the issues mentioned in #106 - thanks for that, and see below when Body field set to 5 rows, for an example:

In a recent 10.1.2 install I am seeing the field widget row settings respected on the Body field but not the Summary field.
Is that expected?

Patch at #101 works fine but only if the element is visible in the viewport, otherwise the calculated height is not correct.
It seems that the cause is that the clientHeight method applied to the 'p' html tag element created to calculate the height of the text area. If the textarea is not in the viewport, the calculated height for 'p' will be 0.
Pretty sure there is a much better solution (reason why I'm not posting a patch), but for now I solved replacing
var height = element.clientHeight
with
var height = element.clientHeight > 0 ? element.clientHeight : 16;
(16px is the most common height for p)
into core/modules/ckeditor5/js/ckeditor5.js

I went ahead and created a patch from the work @puidu did in #113. That worked for us; we have several instances where the initial render of one of the ckeditor5 instances was out of the viewport, and the height when it was revealed was set to 65px. Changing it to 24 gets us close to the 180pc height all of our other ckeditor instances have.

Can confirm that patch #101 works for D9.5.11. :)
Thx for your work :)