In line 66:
$this->derivatives[$definition['id']]['entity_types'] = $definition['targetEntityType'];
But the annotation says that entity_types is an array. The name also suggests that it's an array. I think the code should change to
$this->derivatives[$definition['id']]['entity_types'] = [$definition['targetEntityType']];
There is some code added, perhaps to address the bug, that we might be able to remove when this is fixed.
/**
* Overrides DefaultPluginManager::processDefinition().
*/
public function processDefinition(&$definition, $plugin_id) {
$definition += [
'entity_types' => FALSE,
];
if ($definition['entity_types'] !== FALSE && !is_array($definition['entity_types'])) {
$definition['entity_types'] = [$definition['entity_types']];
}
}
Or perhaps it's better to leave ::processDefinition() in, just in case someone else makes the same mistake?
Comment | File | Size | Author |
---|---|---|---|
#11 | entity-embed-3059394-11.patch | 2.72 KB | oknate |
| |||
#8 | 3059394-8.patch | 2.29 KB | oknate |
|
Comments
Comment #2
oknateComment #3
oknateComment #4
oknateComment #5
oknateHere are two patches for the issue. A one-liner, and a more elaborate one.
Comment #6
Wim LeersLess custom code! 🥳
Will commit if green.
Comment #8
oknateI had eyeballed it earlier and not tested manually. It turns out the base field definition already had the keys 'label' and 'entity_types', so it didn't set those for any of the plugins. This led to some very unexpected behavior. I checked and we can flip it around, so you add the $base_plugin_definition to our custom settings and that works.
Comment #9
Wim LeersD'oh, right!
I agree this new patch iteration matches the original array merging behavior exactly.
Comment #10
Wim LeersNo longer applies now that #2924391: [media entities] Regardless of @EntityEmbedDisplay plugin, Media entities representing a `image/*` MIME type should be able to have a per-embed `alt` and `title` landed :)
Comment #11
oknateHere's the reroll.
Comment #13
Wim Leers🚢