EntityEmbedBuilder removes data- attributes set by other text filter plugins

I like this idea but maybe it's better to add the $context array to $build['#attributes'] rather than replace it?

Wow, this is a huge bug!

Transferred credit from #2628712: Persist the classes specified in <drupal-entity class> onto the rendered entity.

I was thinking first that to minimize change, we should be passing all attributes through except the ones that Entity Embed cares about.

But the existing filter_align, filter_caption and editor_file_reference filters in core do not strip data- attributes … so I think it makes sense for Entity Embed's filter to match this behavior. It also happens to be the simplest.

The change itself is simple enough, the complexity is in updating the tests.

(This should fail tests.)

This should pass.

reroll, as #13 no longer applies

Rerolling, it looks like my merge was incorrect in patch 14.

I have manually tested this using my reroll in #17. Looks good to go. Marking as "Needs review" since I rerolled it. But it could be marked reviewed and tested by the community since it was just a reroll.

Thanks, looks good!

Looks good to me!

Committed and pushed. Thanks!

+++ b/src/EntityEmbedBuilder.php
@@ -99,18 +98,11 @@ class EntityEmbedBuilder implements EntityEmbedBuilderInterface {
+    $build['#attributes']['class'] = isset($build['#attributes']['class']) ? $build['#attributes']['class'] . ' embedded-entity' : 'embedded-entity';

Shouldn't `$build['#attributes']['class']` be an array?

I tested the class out and it makes no difference for the output whether it's converted to an array in EntityEmbedBuilder or not. It might be easier for 3rd parties in an alter hook, and it used to be an array, so it might be more backwards compatible to change it back.

This is how I tested:

Updated full_html format to support "class" on entity embed:

<drupal-entity data-entity-type data-entity-uuid data-view-mode data-entity-embed-display data-entity-embed-display-settings data-align data-caption data-embed-button class>

Then I edited the embed code in the wysiwyg to add class, then switched back to view output within wysiwyg:

<drupal-entity data-embed-button="inline_image" data-entity-embed-display="view_mode:media.inline_image" data-entity-type="media" data-entity-uuid="3a9c30dc-6a3f-42f3-b7d0-e01a14e38292" class="testing123 hello-world"></drupal-entity>

In EntityEmbedBuilder, the context has the attribute as a string, since it pulls it from the code above:

$context = 
array:7 [
  "data-embed-button" => "inline_image"
  "data-entity-embed-display" => "view_mode:media.inline_image"
  "data-entity-type" => "media"
  "data-entity-uuid" => "3a9c30dc-6a3f-42f3-b7d0-e01a14e38292"
  "class" => "testing123 hello-world cke_widget_element"
  "data-langcode" => ""
  "data-entity-embed-display-settings" => []
]

The build attributes that are built from the context also have the class as a string:

$build['#attributes']['class'] = 
"testing123 hello-world cke_widget_element"

This results in the following output of the preview of the entity in the wysiwyg:

<div data-embed-button="inline_image" data-entity-embed-display="view_mode:media.inline_image" data-entity-type="media" data-entity-uuid="3a9c30dc-6a3f-42f3-b7d0-e01a14e38292" class="testing123 hello-world cke_widget_element embedded-entity" data-langcode=""> 
etc.

Whether you convert it to an array in EntityEmbedBuilder or not, the output is the same.

The only thing I would say, is that it used to be an array before going to the alter hooks:

$build = [
      '#theme_wrappers' => ['entity_embed_container'],
      '#attributes' => ['class' => ['embedded-entity']],
      '#entity' => $entity,
      '#context' => $context,
    ];

So it would probably be more backwards compatible to convert this to an array before sending to the alter hook here:

$this->moduleHandler->alter(["{$context['data-entity-type']}_embed", 'entity_embed'], $build, $entity, $context);

Since this issue is marked as fixed, I'm opening a new issue for this:
#3021505: Fix regression introduced by #2917132: Use array for class attribute in EntityEmbedBuilder

thanks @oknate for opening new issue

Here's a re-roll on top of 8.x-1.0-beta2 since a release hasn't came out in awhile.

#3021505: Fix regression introduced by #2917132: Use array for class attribute in EntityEmbedBuilder landed!