Caption should work like the drupalimage plugin (editable in WYSIWYG, not in dialog)

Postponed until it's clear whether we can modify the plugin in core or we'll have to write our own caption plugin.

What's nice about the core-provided image button is that you can use the CKEditor buttons to add bold, italic, and links. It seems like we can still manually write out the HTML for that using the entity embed caption textfield, but not ideal.

Does anyone have any insight as to why we have the caption textfield like we do instead of the checkbox that the image plugin provides?

The editable-in-widget functionality of CKEditor widgets relies on having a simple property on the embed widget definition called "editables". This property defines the DOM elements of our widget that can be edited inline the editor.

I tried setting this property just to see if it would do anything, and it does not. I think this is because we're using AJAX to replace the embed code with the DOM structure we want:

        // Fetch the rendered entity.
        init: function () {
          /** @type {CKEDITOR.dom.element} */
          var element = this.element;
          // Use the Ajax framework to fetch the HTML, so that we can retrieve
          // out-of-band assets (JS, CSS...).
          var entityEmbedPreview = Drupal.ajax({
            base: element.getId(),
            element: element.$,
            url: Drupal.url('embed/preview/' + editor.config.drupal.format + '?' + $.param({
              value: element.getOuterHtml()
            })),
            progress: {type: 'none'},
            // Use a custom event to trigger the call.
            event: 'entity_embed_dummy_event'
          });
          entityEmbedPreview.execute();
        },

I can't say for sure, but it seems like since we're using AJAX like this, the editables feature of CKEditor may just not ever work. I'll spend a bit more time checking that out to be sure.

#2544020 should happen first, see #2544020-8: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin.

If we can make this work, then #2990624: Cannot insert valid HTML tags in captions + test coverage for interaction with core's FilterCaption will be unnecessary.

Thanks to the work I did on #2511404-73: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor, I ended up with #2544020-18: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin. Which made doing what @bkosborne alluded to more than 2.5 years ago actually pretty simple.

This is not yet fully operational, because data is not yet being persisted. But this shows it can be made editable after all, and based on the work in #2544020-18: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin, I am fairly confident I can make this work completely tomorrow.

This is what it looks like:

Forgot the patch without the blocker — that shows how few lines were required to at least make it look like it's working :)

Per #2544020-22: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin, two lines that should've been in this patch snuck into that patch.

Actually, parts.caption doesn't make a lot of sense. Removing it, and documenting why.

This makes what you see in #13's screenshot actually work 🥳

Applies on top of #2544020-22: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin.

Edge cases to test:

• 👎 Triggering "Source" button does not trigger a blur event on the caption editable, which results in the source being inaccurate. The blur event should be triggered.
• 👍 Bold, italics etc work in the caption editable *and* are properly HTML-encoded.
• 👎 Inserting a link in the caption editable fails in the drupallink plugin on this line: var range = selection.getRanges(1)[0];

As of #2544020-25: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin, changing the caption is no longer triggering re-fetching of server-side previews! Rebased on top of that patch.

So … do we keep this in EntityEmbedDialog? We should for now at least; removing it is out of scope for this issue. Some people may be relying on it. Tagging Needs followup for that.

In any case, if you created for example a caption with some emphasized or bold text in CKEditor, it'd show up correctly in the dialog, but upon saving it'd be escaped. This fixes that.

Edge cases to test:

• 👍 Bold, italics etc work in the caption editable *and* are properly HTML-encoded *and* are displayed properly in the EntityEmbedDialog *but* upon saving that dialog they're escaped.

Screenshot to make #19 more clear what I'm referring to.

WRT having the caption editable and making it editable while making the captioned content (such as a Youtube video) unavailable for interaction: see #2544020-28: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin.

Edge cases to test:

1. 👍 Not specifying a caption in EntityEmbedDialog causes no data-caption value to be sent to the client; the client should not fail, and automatically update the preview to be captionless.

This patch fixes that.

+++ b/js/plugins/drupalentity/plugin.js
@@ -104,6 +119,14 @@
+              // And ensure that any changes made to it are persisted.
+              widget.editables.caption.on('blur', function (e) {
+                var entityAttributes = CKEDITOR.tools.clone(widget.data.attributes);
+                entityAttributes['data-caption'] = e.sender.$.innerHTML;
+                widget.setData('attributes', entityAttributes);
+              });

I asked Krzysztof yesterday (see #2544020-27: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin) about this.

He confirmed that .on('change' is not available. He suggested .on('keyup'. That ensured this edge case was solved:

👎 Triggering "Source" button does not trigger a blur event on the caption editable, which results in the source being inaccurate. The blur event should be triggered.

… but it doesn't yet solve the case of the user selecting part of the caption and clicking the "Bold" button, then clicking the "Source" button. Even listening to more events (such as click and paste) won't fix this.

He had two suggestions for fixing this:

1. Listen to the global (editor-level) change events and then check if something has changed in the caption editable. This will result in many unnecessary function calls.
2. Move away from the current side effect-free downcast() event handler that #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin introduced (and which he was both surprised by but also very supportive of) and force it to inspect the DOM for changes. He indicated that this is an unfortunate weakness in CKEditor 4, and there isn't much that can be done about it. This would mean moving the current downcast() logic to a _dataToHtml() helper method, and introducing a _updateDataBasedOnDomChangesForEditables() helper method, so that we'd end up with:
   

   downcast: function() {
     // First, let's make sure this.data also contains changes made directly in the DOM; changes we can't listen for.
     this._updateDataBasedOnDomChangesForEditables();
     // Next, let's do the actual downcast based on this.data.
     return this._dataToHtml();
   }
   

After I wrote that, I realized that there actually now is something we could use: https://caniuse.com/mutationobserver. It has sufficient browser support. It doesn't require such hacks. Let's do it! 🤓

+++ b/js/plugins/drupalentity/plugin.js
@@ -115,19 +115,27 @@
-            editor.fire('lockSnapshot');
...
-              editor.fire('unlockSnapshot');

These changes were accidental. Brought it back.

But thanks to the changes in #23, a clearer pattern now emerges.

Following the lead of #2544020-34: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin, doing the same for captions. That means that instead of showing figcaption, it'll now show Caption.

The above patches don't work well yet in the scenario where no caption is turned on: in that case, this patch results in a JS error. Easily remedied.

Let's test this together with #2544020-40: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin.

Time for a final round of clean-up, then let's land this 🥳

We were only using parts (which is optional: it exists merely for convenient access to DOM elements if a particular widget implementation needs a lot of DOM access) for link integration (#2511404: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor) anymore. And as of #2511404-77: Image entities/fields embedded using Entity Embed cannot be linked in CKEditor, that's no longer using it anymore.
So: time to drop parts altogether!

Also this was using a copy/pasted allowedContent from the CKEditor core codebase, now using the same allowedContent as the drupalimagecaption plugin for consistency.

#2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin just landed, let's test this.

That passed :)

And

Inserting a link in the caption editable fails in the drupallink plugin on this line: var range = selection.getRanges(1)[0];

now also works.

For Needs tests: created #3060396: Add CKEditor Widget JS test coverage.

For Needs followup: created #3060397: [PP-1] Caption in both dialog and CKEditor widget is weird, should we remove the one in the dialog, to match core?.

🚢