Entity embed dialog can no longer use custom data- attributes in CKEditor 5

FYI: this logic must be modified to pass all attributes to the server side: also the GHS ones, and not just the ones explicitly used/supported/consumed by the entity_embed CKE5 plugin:

      this.listenTo(buttonView, 'execute', (eventInfo) => {
        const element = editor.model.document.selection.getSelectedElement();
        const libraryURL = Drupal.url('entity-embed/dialog/' + options.format + '/' + element.getAttribute('drupalEntityEmbedButton'));

        let existingValues = {};

        for (let [key, value] of element.getAttributes()) {
          let attribute = entityEmbedEditing.attrs[key]
          if (attribute) {
            existingValues[attribute] = value
          }
        }

        // Open a dialog to select entity to embed.
        this._openDialog(
          libraryURL,
          existingValues,
          ({ attributes }) => {
            editor.execute('insertEntityEmbed', attributes);
            editor.editing.view.focus();
          },
          dialogSettings,
        )

— https://drupal.slack.com/archives/C01GWN3QYJD/p1703092541221209?thread_t...

P.S. there already is explicit test coverage for additional attributes not getting stripped. So this is NOT a data loss issue! This issue is requesting the ability for the server side rendered dialog to be hook_form_alter()ed to support editing additional attributes.

Test failures on MR are unrelated to the changes made in there and (the test I added specific to this issue is passing, btw). I assume this is due to the welcome addition of testing multiple core branches by the testbot, which has surfaced several pre-existing issues.

@bnjmnm Thanks for the MR! I've confirmed it fixes the issue described, but I did find some bugs:

1. If a wildcard is defined in the Source editing plugin's allowed tags, e.g. <drupal entity data-*>, then a TypeError: attr.key.replace is not a function is thrown.

2. If one of the default data attributes is defined in the allowed tags, e.g. <drupal-entity data-entity-type data-entity-uuid>, then the media doesn't render after embedding. I believe this is because the default key in the attrs object, e.g. "drupalEntityEntityType", gets superseded by the now-automatically-added "dataEntityType", causing some other part of the code to break.

I've attached a patch to your MR that works around these two issues.

For #2, the fix is easy enough: add a check that the attribute is not already defined in the attrs array.

For #1, it's a bit trickier. It appears CKEditor's SchemaItemDefinition.register method does not accept RegExp objects in the allowAttributes property. That means there's no way to simply pass the wildcard along, AFAICT.

Therefore, I simply added a typecheck to simply skip if attr.key is not a string, but this is quite opaque - I'm not sure if it'd be better to actually throw an error instead, so the user isn't confused if their wildcard is not respected. That is, of course, unless you have a better idea of how to handle wildcards.

Thank you @bnjmnm and @odensc for your contributions. After applying the two patches you suggested, everything is working perfectly. For my convenience, I've combined both patches into one.

Using patch #9 with D10.2.2 solved our issue (broken custom attributes)

Thank you!

Re-rolled the patch for the new release of Entity Embed version 1.6
I couldn't push the merge request resolved conflict though,

Rebased branch on 1.7 and regenerated JS.

Thanks for the rebase @mortona2k. I also added the fixes from #9 into the MR.

Hi

Tested latest diff in Drupal 10.5.1 and entity_embed 1.7.0 and our custom data attributes are working.

Attaching the patch here to be included as a composer patch.

+1 RTBC

Moving to RTBC, we've been using this in production for a year+, and there are a few other people here that have confirmed the fix.

I'd like to try and find a way to feed custom attributes into the plugin in a way that doesn't require filter_html, since we don't always use it. I might look into this in a separate ticket.