Discovered while porting the CKEditor Widget to Drupal core in #2994696: Render embedded media items in CKEditor.
The current upcasting logic only inspects the element's data- attributes, not the element name.
if (attributes['data-entity-type'] === undefined || (attributes['data-entity-id'] === undefined && attributes['data-entity-uuid'] === undefined) || (attributes['data-view-mode'] === undefined && attributes['data-entity-embed-display'] === undefined)) {
This means that <foo data-entity-type…> would also be upcasted. That does not match the filter's behavior, so let's restrict it.
Here's a video:
https://www.drupal.org/files/issues/2019-06-26/foo-upcasting.mov
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | interdiff.txt | 627 bytes | wim leers |
| #12 | 3064288-12.patch | 4.46 KB | wim leers |
| #12 | interdiff.txt | 4.03 KB | wim leers |
| #7 | 3064288-7.patch | 3.78 KB | oknate |
| #7 | 3064288-7--TEST-ONLY.patch | 2.49 KB | oknate |
Comments
Comment #2
wim leersComment #3
oknateComment #4
wim leers#3: yep, that's the pre-patch behavior, right?
Comment #5
oknateYes, manually tested the patch, and it works. We could easily add test coverage.
Comment #6
wim leersTest coverage would be lovely actually. That'd be the CKEditor plugin equivalent of
\Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTest::testOnlyDrupalEntityTagProcessed()for the filter plugin.Comment #7
oknateAdding test coverage.
Comment #9
oknateAs a follow-up, we could test for each missing attribute.
Comment #10
wim leers👍
Comment #11
wim leersComment #12
wim leersTest clean-up prior to commit.
BTW, #9: I think that's excessive :) This suffices.
Comment #14
wim leersDrupalCI is apparently down or massively overloaded, because even after >100 minutes it's still queuing 🤔
Don't have time to wait. Ran test locally. #12 introduced a CS violation. Fixed.
Comment #16
wim leers… and d.o apparently lost the interdiff I definitely uploaded in #14. Send hugs to
drupal.org, it's apparently going through a rough time!