Allow contrib to alter EditorImageDialog/EditorImageDialog and have custom attributes be stored

Adding the nod_ summoning tag.

New patch with the allowedContent converted to CKEDITOR.style.

+++ b/core/modules/ckeditor/js/plugins/drupallink/plugin.js
@@ -111,8 +122,17 @@
+        allowedContent: {

missing new CKEDITOR.style()?

drupal/core/modules/ckeditor/js/plugins/drupalimage/plugin.js
  36:13  error  Extra space after key "alt"          key-spacing
  52:18  error  "i" used outside of binding context  block-scoped-var
  52:25  error  "i" used outside of binding context  block-scoped-var
  52:51  error  "i" used outside of binding context  block-scoped-var
  53:61  error  "i" used outside of binding context  block-scoped-var
  60:20  error  "i" is already defined               no-redeclare
  60:20  error  "i" used outside of binding context  block-scoped-var
  60:27  error  "i" used outside of binding context  block-scoped-var
  60:49  error  "i" used outside of binding context  block-scoped-var
  61:55  error  "i" used outside of binding context  block-scoped-var

Other than that, ok with me.

I think this should fix the remarks in #12.

Would be better to rename the second iteration variable, that would solve the var eslint errors.

(duplicate)

New patch, got no eslint errors here.

EDIT: also removed a .trim() since the .split() regexp already takes care of that.

good to go.

CKEditor dialog are not alterable

This is plain wrong. You can totally alter the image form. Because it's a form like any other.

What is missing, is the ability for any additional (custom) attributes that those form alterations may add, to persist on the images. Which means this is JS land, and we cannot write test coverage for this (removing the Needs tests tag because of that).

I very much support this, but it should be manually tested. Given that @nod_ RTBC'd it, I'm assuming he did the necessary testing.

#20: np np :) I feel very strongly about unambiguously wording things, I know many people don't feel that strongly about that.

Many thanks for doing this!

This seems like a very sensible "simplest thing that can possibly work" in 8.0.0 so we can make this whole are shine more in 8.1.x. Great work.

Committed and pushed to 8.0.x. Thanks!

That fix had side effects: it unlikely fixed #2578957: (regression) "Open in new window" checkbox does not work anymore

This caused a nasty regression: #2585173: [regression] "Allowed HTML tags" setting corrupted upon accessing Text Format configuration UI. Seems like @nod_ did not do the necessary manual testing :(

FYI: Just uploaded a patch over at #2585173: [regression] "Allowed HTML tags" setting corrupted upon accessing Text Format configuration UI. Should fix the issue.

I'm getting strange JS errors after this:

Uncaught TypeError: Cannot set property 'float' of undefined

Looks like styles is not initialized, but I have no idea what's different in my case that it only happens for me?

This for example happens when switching from an text format without drupalimage to one that has it enabled and completely breaks wysiwyg then.

The following fixes it for me, I can open a follow-up tomorrow, but I don't know enough JS to know if the problem is really somewhere else?

diff --git a/core/modules/ckeditor/js/plugins/drupalimage/plugin.js b/core/modules/ckeditor/js/plugins/drupalimage/plugin.js
index 724fa50..5160797 100644
--- a/core/modules/ckeditor/js/plugins/drupalimage/plugin.js
+++ b/core/modules/ckeditor/js/plugins/drupalimage/plugin.js
@@ -43,6 +43,7 @@
         });
         var allowedContentDefinition = {
           element: 'img',
+          styles: { },
           attributes: {
             '!data-entity-type': '',
             '!data-entity-uuid': ''

Another regression: #2598070: [regression] CKEditor Link button does not show if HTML filtering is enabled.