Problem/Motivation

This issue is to fix a change of API in EntityEmbedBuilder.php

It used to be that $build['#attributes']['class'] was an array. In #2917132: EntityEmbedBuilder removes data- attributes set by other text filter plugins it was changed to a string. For people using the alter hook to add a class, this can lead to a fatal error. See https://www.drupal.org/pift-ci-job/1151136

Proposed resolution

For backwards compatibility, and for ease of alterability in alter hook ["{$context['data-entity-type']}_embed", it should be changed back to an array.

See this comment for more info.

CommentFileSizeAuthor
#17 3021505-17.patch2.39 KBwim leers
#17 interdiff.txt822 byteswim leers
#12 entity-embed-beta2-3021505-12.patch1.74 KBygannett
#5 entity-embed-2917132-should-fail.patch1.65 KBoknate
#4 entity-embed-3021505-4.patch2.41 KBoknate
#3 entity-embed-3021505-3.patch917 bytesoknate

Comments

oknate created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new entity-embed-3021505-3.patch917 bytes
oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new entity-embed-3021505-4.patch2.41 KB

Adding test coverage.

oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new entity-embed-2917132-should-fail.patch1.65 KB

Adding a patch for testing only, should fail. This should show an error "Error: [] operator not supported for strings in entity_embed_test_entity_embed_alter".

oknate’s picture

oknate
Location Greater New York City Area
commented
Priority: Minor » Normal
Status: Active » Needs review

Please review patch #4

oknate’s picture

oknate
Location Greater New York City Area
commented
Assigned: oknate » Unassigned
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
amme’s picture

amme commented

i confirm that `$build['#attributes']['class']` is array again
and my custom alter hook works again

thanks @oknate

amme’s picture

amme commented
Status: Needs review » Reviewed & tested by the community
ygannett’s picture

ygannett commented
Version: 8.x-1.x-dev » 8.x-1.0-beta2
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new entity-embed-beta2-3021505-12.patch1.74 KB

We found entity-embed-3021505-4.patch doesn't apply to 8.x-1.0-beta2. I took the work of the patch from comment #4 and rerolled it. It works with beta2. Please review and test it.

amme’s picture

amme commented

Patch must be applied to dev branch, not to release branch @ygannett

oknate’s picture

oknate
Location Greater New York City Area
commented

If the patch is working for you, please mark issue as RTBC. It speed up the process of the maintainers adding to dev branch.

chris burge’s picture

chris burge commented
Status: Needs review » Reviewed & tested by the community

#4 corrects the regression that was introduced in #2917132: EntityEmbedBuilder removes data- attributes set by other text filter plugins. It applies to the dev branch. Setting back to RTBC.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Use array for class attribute in EntityEmbedBuilder.php » Fix regression introduced by #2917132: Use array for class attribute in EntityEmbedBuilder

Committing #4, because it contains tests, and #5 proved that those tests are adequate (also tested locally). 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new interdiff.txt822 bytes
new 3021505-17.patch2.39 KB

One nit to fix.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

🥳

  • Wim Leers committed 6ae55ae on 8.x-1.x authored by oknate 
    Issue #3021505 by oknate, Wim Leers, ygannett: Fix regression introduced...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.