CKEditor 5 doesn't save updated value if form submitted right after the change

In MR!5127 debounce is changed to how it was before https://www.drupal.org/project/ckeditor5/issues/3229174 →
The only thing that changed is "immediate" set to true, so the first execution will be instant. With that changes made to WYSIWYG and source are always submitted.

But I'm not sure if debounce is needed at all, callbacks (web/core/modules/ckeditor5/js/ckeditor5.js:19) store only one function for each CKeditor instance and it is used by Drupal.editorAttach. So callback is only worth executing once and it can be removed.

There is also Drupal.editors.ckeditor5.attachInlineEditor that uses that callback but I don't see where that attach is used at all.

Different behaviour in Drupal.editorDetach, when there are 1 or more text formats, seems weird, but I also don't understand why we need to "Restore the original value if the user didn't make any changes yet", like if there are no changes, that means field already contains original value?

I believe we do not need a test for the debounce/submit issue since there are already failing tests because of that in https://www.drupal.org/project/drupal/issues/3382780#comment-15289456 🐛 [drupalImage] When ckeditor5_arbitraryHtmlSupport is on, fails to update Postponed

Thanks for reporting, could we get a test case showing the issue.

tests have been added and test-only feature already ran

There was 1 error:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test::testSave
Behat\Mink\Exception\ExpectationException: The string "Very important information" was not found anywhere in the HTML response of the current page.
/builds/issue/drupal-3396742/vendor/behat/mink/src/WebAssert.php:794
/builds/issue/drupal-3396742/vendor/behat/mink/src/WebAssert.php:324
/builds/issue/drupal-3396742/core/tests/Drupal/Tests/WebAssert.php:540
/builds/issue/drupal-3396742/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php:849
/builds/issue/drupal-3396742/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 11, Assertions: 182, Errors: 1.

Issue summary appears complete (thanks for that)

LGTM

I'm worried this reintroduces the bug that #3229174: Making changes directly in the source view drops the made changes on save → fixed.

This needs a review from @lauriii, who worked on the original issue.

Was it verified that removing this does not cause that bug to return?

The issue with source editing not being saved is basically the same as this one.
The difference is when in "source editing" mode change:data event is called only when switching back or if the form is submitted.
So if only the source was edited and the form was submitted without switching mode back, that 400ms delay mentioned in this issue has no chance of being executed.
Fix from https://www.drupal.org/i/3229174 → were removing delay only for source editing.
With a fix from that issue in either mode on:change has no delay on the first input and everything is good.

I've confirmed manually that the bug from #3229174 has not been reintroduced.
It is testable I believe, but it will be the same test as provided in this issue, only with the mode changed.

It is testable I believe, but it will be the same test as provided in this issue, only with the mode changed.

Could you expand the test to cover that too? 🙏 Then we'll never have to worry about this again! 😄

Expanded test to include only source editing changes.

It shouldn't fail even without changes from this issue, the one way to make it fail is to bring back the code mentioned in #8 but skip calling callback()

Attempted to replicate the issue multiple times on Windows 11 Pro using the latest version of Chrome, but it appears that the issue cannot be manually reproduced.

Rebased and verified test failure.

Also was not able to verify manually but the test seems to cause it. Can we confirm though nothing has broken?

Still needs submaintainer sign off as not familiar with that js file.

I still can manually reproduce the issue on the latest 11.x
So for me, everything remains the same and nothing new is broken.

If it helps, the exact way I reproduce is:
open edit, focus CKEditor field. Place the cursor on the save button. Type something and click save right after. As described in the Issue summary there is only a 400ms window to do that (probably a bit less, including input lag, code execution order etc.)

However, the functional test hits that window consistently, which is why this issue was reported.

Going to unassign it. Been 3 months and haven’t been able to get ahold of laurii who’s probably busy, and hate to see the issue stale

@Wim Leers do you see any issue in the MR as someone who’s done a lot with ckeditor?

1) Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test::testSave
Behat\Mink\Exception\ExpectationException: The string "Very important information" was not found anywhere in the HTML response of the current page.
/builds/issue/drupal-3396742/vendor/behat/mink/src/WebAssert.php:794
/builds/issue/drupal-3396742/vendor/behat/mink/src/WebAssert.php:324
/builds/issue/drupal-3396742/core/tests/Drupal/Tests/WebAssert.php:540
/builds/issue/drupal-3396742/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php:860
/builds/issue/drupal-3396742/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 11, Assertions: 182, Errors: 1.

Test-only feature is still showing the coverage.

Wim Leers → credited nod_ → .

I'm not very familiar with this area, but given

1. the test-only CI job failing on the new test 👍
2. @nod_'s comment at https://git.drupalcode.org/project/drupal/-/merge_requests/5127/diffs#no...

RTBC seems appropriate 👍

The issue summary and comments make this problem easier for me to understand, thanks! The fix looks to me like the correct thing to do also.

Backported to 10.2.x as an eligible bug fix.

Committed and pushed 5d5dde4357 to 11.x and 4548f1308c to 10.3.x and 5540b5f84a to 10.2.x. Thanks!

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

This introduced a regression: 🐛 [regression] Drupal.editors.ckeditor5.onChange event doesn't fire after final input if user types too fast. Active .