Also use text editor (CKEditor) for "summary" of a text field

So will the editor on the summary change if you change the format of the body ?

#6 It could and should not, because the summary can be edited independently of the actual body.

I am looking into this issue.

Any news here?

I've said this before: we should've gotten rid of the "text with summary" field instead. If you want a summary, create a separate field.

Isn't it better to fix this and discuss such changes of the core in a separate issue for 9.x?

No progress in almost a year, I'm unassigning it.

#13: Probably. If you work on a patch I'd be happy to provide reviews.

@Wim Leers, @DamienMcKenna. I've run into an issue with using this patch which I would love to solve to move this along. Since the summary is considered an instance of StringInterface when it's validated
here: core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php:35, it fails due to these two validation issues:

!is_scalar($value) && !($value instanceof MarkupInterface)

with the following error:

This value should be of the correct primitive type.

This would be a nice addition, I'd just like to mention that usability-wise we'd be hitting #2877527: Make CKEditor (approximately) reflect the rows setting of a field as the summary and body textareas end up having the same height.

About having two separate fields like suggested in #12, some related issues are #1529188-15: Provide separate text format for summary in long text with summary fields and #1378350: Clean up the "Long text and summary" field but I have not found one which addresses the problem of the synchronization between these two separate fields to cover the case when the summary is empty.

Tried patch from #4 and get error as mentioned in #18. I attempted several hacks and none worked. I even tried to create new field type just for summary field, that also did not work. For me D8 looks more resistant to hacks.

Now I have patch for core editor module. It is mostly on JS side in editor.js and some in Element.php.

Idea is to attach editor to more than one textarea same time. As you see in patch for Element.php. It add ID of summary field as well if summary field is present. modification in editor.js is to make it manage more than two editor same time for single format selector.

There is one issue with this patch. Patch will not work if format is changed by selecting "Format" select box. But that does not matter in our case as on all our sites we are using single text format per textarea field. So, not essential for us to fix it at this moment.

Patch #23 doesn't work for me :(

@ivnish did patch apply cleanly?
Also, are you getting any JS error in browser console?

Patch applied successful. No js errors in console.

I will try this patch on fresh drupal site

@ivnish Just confirmed the patch is working on fresh D8.

1. Downloaded latest Drupal 8.7.5 from https://www.drupal.org/project/drupal/releases/8.7.5
2. Made a regular installation with "Standard" profile. (No composer, no drush. Just old way using browser)
3. Put patch on D8 root folder and applied.

Got it working straight away as in this screenshot .

On my site I have only one text format. Can it affects?

@ivnish That is perfect site for this patch. The patch will work fine for your site.

Also, you may have more than one text format in your site but only one text format to be allowed on a field to make this patch work on that field.

We have applied the patch to a Drupal 8.7.5. It is not a clean/new install.
We also patched an almost bare 8.7.6 install (had a few contributed modules installed.)
We then reinstalled that bare 8.7.6 as an 8.7.7 to make it a clean install.
The patch is not working on anything but the clean install.
Why is it not working on the modified sites?
We are asking this because we can't start the site over.
If we know the reason for requiring a cleanly installed site and what might be wrong when installing on a modified site we can try to find a solution.

@Chris, there is no requirement for clean installed site for the patch from comment #23 to work.

In fact we are using this same patch in our production site that exist since Drupal 5 or 6. Currently it runs on 8.7.4. Later I will try to update here if patch continues to work after updating site to latest D8.

I assume, there might be something in your site that conflicts with the patch.

Patch in #23 worked for me on 8.7.8 on my existing site (not fresh install).

Patch in #23 worked for me too on D8.7.8

Commit please

Thanks for the folks who've worked on this so far. I agree this would be a good improvement.

@TD44: Comment #36 is entirely unhelpful. There's nothing to commit. Patch #23 causes existing tests to fail, and has no tests of its own. This isn't even in 'Reeds review', much less 'RTBC'. Core committers should not waste their time in here until the community gets something actually commit-able ready, first.

Tagging for 'needs tests' (new tests to prove the new functionality works).

Still 'Needs work' status b/c of the existing test failures in https://www.drupal.org/pift-ci-job/1425970

Cheers,
-Derek

On Drupal 8.9.3, the patch in #23 applies without errors, but it no longer works.
it generates java-script errors like:
Uncaught TypeError: field.each is not a function

web/core/modules/editor/js/editor.js on line 187 >> field.each(function() {

Reroll patch for 9.1.x. Please review

@all: Please don't set this issue to 'Needs review' or say "Please review" until:

a) The existing test failures are fixed. You can run these tests locally and get them working before you upload a patch.
b) You add new tests (that pass) that verify the new functionality.

If you fix (a) but don't have the time/knowledge to solve (b), that's totally fine. Please upload your patch once you've gotten the tests passing locally, with an interdiff relative to the latest patch. You can manually trigger testing for your patch, even if the issue is not in 'Needs review' status. Then we can all see that the existing test failures are resolved, which would be big progress! But there's no sense "please review"'ing this issue until the current tests are passing, we also have new tests, and can accurately remove the 'Needs tests' tag.

Other things you can do to move this forward that would help get this out of 'Needs work' status:

1. Create before screenshot of a summary from a text area, showing the existing UI. Attach that to the issue and embed it in the User interface changes section of the summary to replace the @todo I just put there. Might be worth updating the 'After' screenshot, too, so that we see exactly the same text area before/after. Once that's done, you can remove the 'Needs screenshots' tag.
2. Update the summary to fully describe the proposed change here, removing the other @todo items I'm adding here. Once complete, remove the 'Needs issue summary update' tag.

Thanks!
-Derek

Apologies, was trying some hands -on on Community for first time , will take care going forward . Thanks for your note

@jayprakash01: No worries! Thanks for moving this forward, and apologies if I came across harshly. Not at all intended. Was trying to clarify how we use the Status and 'Issue tags' fields to keep things organized, and to provide action items that'll help.

Cheers,
-Derek

p.s. I'll add that there's also no need to queue a patch for testing on multiple platforms/configurations except in very rare circumstances (e.g. you reasonably expect it to be PHP version or DB dependent). Certainly not worth it if there are known test failures. Each test run takes ~1.25 hours of AWS time, which the Drupal Association pays for. So we try to be mindful of that cost and only have the testbot do work that's really necessary. Thanks again, and welcome to Drupal core development! :)

Fixing test.

The D8.9.x version, but it still doesn't work when trying to change the text edition format when you edit the node...

@dww I also face issue mention in #39 for drupal 9.1.x, I have done some changes now issue has been fixed, Please review and advise.

I was looking to repo of drupal project and found latest commits related to this issue.
I hope i can get friendly support to get familiar with core contribution and writing tests.
What i can say looking to dispatcher while process was performing for latest patch posted in #47 that it fails Tests for CKEditor
FATAL Drupal\Tests\media\FunctionalJavascript\CKEditorIntegrationTest: test runner returned a non-zero error code (2).
I think is a good point to enter for me into whole process of contribution to core, performing local execution of Unit Tests and other related stuff like fixing, debugging, patching, gitting.

I am assigning this issue to me hopefully that i can get support from @vsujeetkumar
May i ask about mentoring me or pair programming somebody to investigate perform and fix this issue?

Fixing Test

@dww, Issue has been fixed mentioned in #39, Also test, Please review.

I have queued test for branch 8.9.x-dev. @vsujeetkumar, as far as i understand this issue is no longer exists in drupal version 8, so this patch is not applicable to branch 8.9.x.

Then only manual testing is requres to make sure this fix could be commited to core.

Hey folks, do you think it's a good idea to change this without possibility to opt-out? We (and I think we're not alone) rely on summary having no html code and it would break our code. I also noticed on screenshot that there is no possibility to change text format. So is text format inherit from main body field? I think this would be confusing from UX perspective.

Applied patch 52 . Now the summary field is also appearing as a WYSIWYG Editor
before patch:

after patch:

Tested #52 on 9.1, Summary field is appearing with an Editor after the patch
This can be moved to RTBC

Hi,

#52 failed on 8.9.x.

Baher

Hi,

I create this patch 2671162_58.patch based on the patch 2671162-after_patch-52.patch for 8.9.x. Can you tested.

Baher

Hi,

Sorry the last patch is missing a file. It will work on 8.9.x version.
You can test this one.

Baher

Re-rolled the patch for 9.2.x

The patch #60 looks good for me on a D8.9.8

Patch #62 works fine for me on a D9.1.0

#67 is working for me on Drupal 9.1.3

I have reviewed and found some JS issues, Now it has been fixed along with some tests and cs issues, Also #67 is not applying on 9.2.x, So I have created interdiff with #62 because its working patch for 9.2.x. Please have a look and advise.

I can't get either of the last two patches to apply to 9.1.6. :(

I recreated the patch, which applies on 9.1.8 as well

Fixed custom command fail issue for 9.1.x, Please have a look.

Fixed fail test for 9.1.x, Please have a look.

Patch #79, seems fine to me. Adding before and after patch screenshot for reference.

After #79, tags are available as per the body section.

Updated IS

Patch #79 applied cleanly working as expected , sharing screenshot for reference.

Patch at #79 doesn't seem to be working for Drupal 9.2.4. Patch applied no problem, but still no CKEditor on Summary field. Am I missing something?

** Ignore that - just updated to 9.2.5 and it's working. Thanks for the patch.

The patch applied clean to 9.2.6 but does not change the summary field to CKEditor

#79 worked for me on 9.2.7

drupal 9.2.8 patch#79 didn't apply

or did I do a mistake:
"drupal/core": {
"wysiwyg_in_body_summary": "https://www.drupal.org/files/issues/2021-05-06/2671162_79.patch"
}

Here's a reroll for 9.3.x and 9.4.x.

I am making screenshots.

I can confirm the patch is working as expected for me
9.3.2

2671162-89.patch was applied to Drupal 9.4.x. I can confirm that the patch does not function as intended after applying the patch, I can confirm that the patch does not work as expected.
For your convenience, I've attached the Before and After Patch Screenshots.

Before Patch Screenshot:

After Patch Screenshot:

2671162-89.patch was applied to Drupal 9.4.x. I can confirm that the patch does not function as intended after applying the patch, I can confirm that the patch does not work as expected.
For your convenience, I've attached the Before and After Patch Screenshots.

Applied #89 patch in drupal-9.3.x-dev
After patch i don't see any changes i am getting same result before and after patch
for ref sharing screenshot ....

yes, it works in 9.3 but apparently not in 9.4

I confirmed it works on 9.3.x

here is my attempt at re-rolling that patch for both 9.3.x and 9.4.x

ok, those last patches had typos, I fixed them

missed removing that typo in the 9.3.x patch file

#79 is the only patch i have been able to get to apply to 9.2.11; and sadly it doesn't do anything.

Also, as part of this effort, shouldn't body/summary allow for different editors? I guess in core the best we could hope for is setting default editor for each and would need to modify something like Better Formats module to limit each to specific editors.

As an example use case of this; for the Summary i want a very limited editor with specific character count limit (CK plugin).

hmm.. oddly enough it seems to be working now. #79 patch and 9.2.11 - but still need to specify different editors for summary/body - but this looks like a great place to start. thanks.

Here's a reroll of #89 against 9.4.x with some fixes for the CkEditor5 tests.

#108 is working in Drupal 9.4.0.

Hmmm #108 is working for me on one site, and not working on another. The one it's not working on has 20 other core patches, but then none of them have anything to do with CKEditor.

Patch #108 does not apply anymore with Drupal 9.4.8...

Patch re-roll to apply on the latest 9.5.x and 9.4.8

@cslevy your patch re-roll works. I owe you a beer. Thank you!

Patch #114 applies now.

Here's a patch for 9.5.x

Have confirmed that #116 works well on 9.5.x.

There is some test coverage included in the patch, so is additional test coverage required?

#116 is working in Drupal 9.5.2

Thank You!!

Does any one have solution for this issue ? I want to use ckeditor5 in summery field in drupal 9.

I applied #2671162-116: Also use text editor (CKEditor) for "summary" of a text field on 11.x and manually edit the editor.js file as the es6 file was removed.

Also, some of the test files modified on the patch there were removed in #3270438: Remove CKEditor 4 from core so leaving the other tests.

EDIT: Applies OK to 10.0.9 and likely 10.1.x as well

latest patch seems not to apply anymore

re-rolled and created a new MR from it ...

let's see what happens.

Also for open tags

Fixed linting error in editor.js file.

Do we have a patch who is working well for the last drupal 9 version?

I just tested the patch #122 in drupal 10, and it's working well

Tested #122 with 10.1.3 and works as intended.

wrong post I can't delete, sorry.

#122 rerolled.

I just tested it again on drupal 10.1.8-dev and it's not working anymore, do you know when this will be included in core?

did someone have a patch working for 10.2 at least?

I'm also curious about the status of getting this in Drupal 10. Any chance to get a patch rolled to 10.2.2?
EDIT: Actually, the 11 patch shared in #132 seemed to apply just fine on 10.2.

#132 applies cleanly on drupal 10.2.2.

CKeditor didn´t show up until I turned on "Always show the summary field" in content type´s form display for the body field.

Is that intended?

Supplement to my comment #136:
I just realized that for new nodes CKeditor shows up without the checkmark as mentioned above.
But for existing nodes I have to do that.

Edit: It looks like it doesn't work the same for all content types for me. What I had reported so far (that I had to check the box) applied to website. For all other content types (all self-created), the CKeditor also appears without any further steps).

I applied patch #132 to Drupal 10.2, but nothing changed because in the editor.libraries.yml file, drupal.editor has the version set as VERSION. Therefore, I rerolled the patch and removed it. If this is incorrect, please don't hesitate to provide the correct solution.

made the MR green "again" after some updates from 11.x
A lot of errors on tests ... Needs review, for now ... so maybe there could code improvements - code review.

Wondering if we still need the "Needs tests" tag ...

Tagging for usability.

My only concern is this feels odd. You essentially have two full text fields now so why not add 2 fields.

But will leave for usability

Also going to tag for framework thoughts.

First of all thanks to everyone for the work!

But notes I'm seeing

Will need a configuration on the field for this feature, should not be the default added.
Will need to be able to limit the formatters, separate from the feature that was already added to filter formats from the main field. I imagine people won't want Full HTML to be used in their summary.
Will need a formatter configuration that will filter the summary now as the "Summary or trimmed" formatter don't believe is any longer safe.

Personally I'm wondering if this belongs in a contrib module vs core.

As sub maintainer I'm kinda -1 but this issue pre-dates me putting that hat on so will defer to the framework managers.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I applied #138 patch and it worked but there was an issue with unrecognized setAttribute function inside an event handler that casued an error and max length not working, setAttribute is a method that only works on individual DOM elements, not on jQuery objects or collections of elements.

To fix this, I modified the event handler to iterate over each element in field using .each(), allowing us to correctly call setAttribute on each raw DOM element individually.

I'm truly grateful for the efforts here, but ideally, I would like to be able to have this option on any full-text field - not just "body."

The 10.2 patch (132) works well for Body fields. Is the ultimate D11 fix also just going to target the body field?

It now just works for body fields??

The original patch I posted in #23 were supposed to work with any long text field with summary. But I see it now evolved lot more but restricted to just body fields as there are some hard coding for body fields. Definitely it needs to be a generic one.

Updated the MR - fix tests. made it green

I think this was in "Needs Review"

About the latest comments ... it's NOT only about the body field.... it's a generic solution for all text with summary fields.
Maybe the confusion is from tests ... that uses the already existing body field.

Believe this should be postponed on #3427095: [Meta] Deprecate text_with_summary

let's have https://www.drupal.org/project/drupal/issues/3427095 linked ... at least

@smustgrave:

When you tag an issue for usability review, please make it easy for the usability team to review the issue. Update the issue summary:

• The "Proposed resolution" section should describe all the changes made in the issue.
• The "User interface changes" should show the existing UI and the proposed UI.
• The "Remaining tasks" should include one explaining the usability issue(s).

Most of the time, I prefer to have plain text in the "Proposed resolution" section and screenshots in the "User interface changes" section. For this issue, (1) needs work (already noted in the "Remaining tasks"), (2) is already in good shape, and (3) is missing.

You can also attend the weekly usability meeting to present an issue.

I am setting the status to NW.

Usability review

We discussed this issue at #3449637: Drupal Usability Meeting 2024-05-31. That issue will have a link to a recording of the meeting.

For the record, the attendees at the usability meeting were @benjifisher, @rkoller, @simohell, @shaal, and @SKAUGHT.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

The only usability issue we see in the comments is from Comment #140:

My only concern is this feels odd. You essentially have two full text fields now so why not add 2 fields.

As far as we can tell, that is out of scope for this issue. It is certainly relevant for #3427095: [Meta] Deprecate text_with_summary, but in the context of this issue, we have two text fields and they both use the same text format. It makes sense to use CKEditor for both of them.

We also discussed whether it was worth the effort to discuss this issue, considering #3427095. We decided it is worth our time: if the text_with_summary is deprecated in Drupal 11 and removed in Drupal 12, that means it will still be around for several years.

We only tested the updated code on the Body field during the meeting, and I just noticed Comments #144, #145. Personally, I agree: if we are going to make this change, then it should apply to all text_with_summary fields, not just the body field.

Patchs does not works for D10.3.3...

Re-rolled for 10.3.3. Did not include change from #138 as not sure whether that is required. Need to test to see if it works.

I just verified that the patch in #151 works fine in 10.3.3

#151 works fine on D10.3.3 for me too, thanks

fwiw #151 works against 10.3.5, thank you.

#151 applied on 10.3.2. It works : the summary is shown in CKEditor but 1 file does not exist in 10.3.2 (core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5MarkupTest.php) and 2 changes are rejected (core/modules/editor/src/Element.php)

$ ls -l core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5^C
$ patch -p1 < core/modules/editor/js/2671162-151.patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5MarkupTest.php b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5MarkupTest.php
|index bd58595b12..b1a80fc7fd 100644
|--- a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5MarkupTest.php
|+++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5MarkupTest.php
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.
1 out of 1 hunk ignored
patching file core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php
Hunk #1 succeeded at 157 (offset 75 lines).
Hunk #2 succeeded at 319 (offset 75 lines).
Hunk #3 succeeded at 563 (offset 75 lines).
patching file core/modules/ckeditor5/tests/src/Traits/CKEditor5TestTrait.php
patching file core/modules/editor/js/editor.js
patching file core/modules/editor/src/Element.php
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n] y
Hunk #1 FAILED at 67.
Hunk #2 FAILED at 77.
2 out of 2 hunks FAILED -- saving rejects to file core/modules/editor/src/Element.php.rej
patching file core/modules/editor/tests/src/Functional/EditorLoadingTest.php
Hunk #1 succeeded at 182 (offset 2 lines).
Hunk #2 succeeded at 223 (offset 2 lines).
$

#151 does not apply on 11.2.3. What is the plan for Drupal 11?

Adding patch for Drupal 11.1.2 but couldn't add tests code due to shortage of time.

Problem/Motivation

On Drupal 10.6.2, in `CKEditor5TestTrait`
(https://git.drupalcode.org/project/drupal/-/blob/10.6.x/core/modules/cke...)),

The `getData()` method accepts a parameter, but changes passed to it are not applied correctly. I have updated the patch to fix this behavior.

I just verified that the patch in #159 works fine in 10.6.2. However there does still appear to be an issue when "Always show the summary field" is selected in the form display options for the Body field. In that case, the summary still does not have the CKEditor enabled.

here is an updated patch that is working for 11.3.x - re-rolled from the patch in #157

#161 works fine on D11.3.6, thanks!

Does someone want to turn this into a contrib module?

I doubt it'll ever land in core as the consensus from the core members I've talked to is if you get this scenario it would be better to just have 2 separate fields.