Trigger event when Text Editor is attached

Fixed failed commands on #2
Attached interdiff patch

I'm fine with doing this, but we should not do this in a CKEditor 5-specific way. We should add events to:

  Drupal.editorAttach = function (field, format) {
    if (format.editor) {
      Drupal.editors[format.editor].attach(field, format);
      Drupal.editors[format.editor].onChange(field, function () {
        $(field).trigger('formUpdated');
        field.setAttribute('data-editor-value-is-changed', 'true');
      });
    }
  };

+1 to making it not cke5 specific.

We should update the API so that editor attach methods should return a promise, and trigger an event once it's resolved.

What do y'all think about this patch? It tweaks Drupal.editors[editor].attach so it expects a promise and then dispatches an editor:attached event.

I was doing a really odd work around before finding this issue.

        const observer = new MutationObserver((mutationsList) => {
          mutationsList.forEach((mutation) => {
            if (mutation.attributeName === 'data-ckeditor5-id') {
              var waitForEditor = setInterval((ckeditor5Id, textArea) => {
                const editor = Drupal.CKEditor5Instances.get(ckeditor5Id);
                if (editor) {

                  // Do stuff ...

                  clearInterval(waitForEditor);
                  observer.disconnect();
                }
              }, 50, mutation.target.dataset.ckeditor5Id, mutation.target);
            }
          });
        });
        observer.observe(field, {attributes: true});

Custom commands...

😬

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 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.

should be 11.x even

+++ b/core/modules/editor/js/editor.js
@@ -297,7 +297,18 @@
+      const editorAttachPromise = Drupal.editors[format.editor].attach(
+        field,
+        format,
+      );
+      editorAttachPromise.then(
+        (editor) => {
+          $(document).trigger('editor:attached', [editor]);
+        },
+        (error) => {
+          console.log(`Failed to attach editor.\n${error}`);
+        },
+      );

I'd do away with the editorAttachPromise variable and simply call .then() after the .attach().

Also it would be better to use a catch instead of the second parameter of the then method.

Wow, that was quick! Thanks for reviewing! Yes indeed, that is more concise. 👍🏻

Could we add some test coverage for this new feature?

Also could some of the missing pieces of the issue summary be updated just to help the review

Thanks!.

I just opened MR, so it is easier to manage, review, and update the code. It contains same changes as the patch in #13. I just could not apply the patch for the latest D10, so re-rolled it in MR.

It looks like some extra white space was added when re-rolling #13 into an MR.

Removed the whitespaces mentioned in #18 and rebased the branch on 11.x

Was able to spin up an instance using the MR and write a custom module that ties into this event. Seems to work great!

((Drupal, $) => {

  Drupal.behaviors.testEditorAttachment = {
    attach() {

      console.log('behavior attached');

      $(document).on('editor:attached', function (event, editor) {
        if (typeof editor !== "undefined") {
          console.log('Event works!', event, editor);

          editor.setData('<p>Hello world!</p>');
        }
      });

    }
  };

})(Drupal, jQuery);

It's wishful thinking, but it'd be nice if this used native web Event Listeners rather than jQuery events, to reduce another need for jQuery in the large scheme of things, but it works as expected.

I won't mark as RTBC since this needs tests, but this is definitely something I could use in my current projects.

Would it be possible to roll this for 10.1? Is there a reason it must wait til 11?

Re-rolled patch to work with Drupal 10.1.7

The current patch direction does not account for editors that do not return a Promise. This results in a JS error, like that below:

Uncaught TypeError: Drupal.editors[format.editor].attach(...).then is not a function

Example: Try creating an Ace Editor text format and then switching to it, and then try switching back to a CKEditor 5 type format.

Wrapping with Promise.resolve() should help:

Promise.resolve(Drupal.editors[format.editor].attach(field, format))

Re-rolled for Drupal 10.2.3 with Promise.resolve() change per #23

Can we get these changes back into the MR? Core doesn't really use a patch workflow any more.

Fixed up MR and merged #24 back into it. I've hid all the patches.

Thanks for turning to an MR.

Moving to NW for the tests and issue summary update.

The patch #24 does not work with 10.3.1. I have recreated the patch to ensure it is compatible with version 10.3.1.

Here's testing that the editor:attached event is firing.

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.

Oh dear...

The branch seems pretty good to me, but the bot doesn't like it. I probably should not have rebased against the current 11.x? I'm not sure what to do about that. 😬

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.

Tried to fix merge conflict in existing MR and rebase 2000+ commits. Created new MR (did not mean to) but since it seems okay, made it the working branch. Hid the other branch. Test-only test fails:

PHPUnit 11.5.39 by Sebastian Bergmann and contributors.
Runtime:       PHP 8.4.12
Configuration: /builds/issue/drupal-3319358/core/phpunit.xml.dist
F                                                                   1 / 1 (100%)
Time: 00:17.972, Memory: 8.00 MB
There was 1 failure:
1) Drupal\Tests\editor\FunctionalJavascript\EditorCreatedTest::testEditorAttachedEvent
JavaScript condition met:
window.editorAttached === true
Failed asserting that false is true.
/builds/issue/drupal-3319358/core/tests/Drupal/FunctionalJavascriptTests/WebDriverTestBase.php:177
/builds/issue/drupal-3319358/core/modules/editor/tests/src/FunctionalJavascript/EditorCreatedTest.php:133
FAILURES!
Tests: 1, Assertions: 7, Failures: 1.
Exiting with EXIT_CODE=1

Since we have test coverage, remove 'Needs tests' tag.

Typically really should stick with the existing branch if it was correctly pointed to 11.x and already been through a few reviews.

@smustgrave Yes, sorry. It was clear I had to handle the conflict locally. I initially set git config pull.rebase false for the git branch but when I tried to fix the conflict using this merge setting I got a huge number of files needing to be fixed. So I changed to git config pull.rebase true which means rebase not merge? But then I had problem pushing the branch. Ended up creating a new branch without wanting/ meaning to. Still not sure where I went wrong. But git log showed that the latest 11.x commits instead of one in July 2025 as the HEAD

Slightly shell-shocked, hoped it would be easier. I have compared the 'changes' between old and new branches. There seem to be 3 changes in each, seem to be the same. I did also pull my local 11.x prior to rebase.