While working on the port of the Entity Embed filter's test coverage to core in #2940029: Add an input filter to display embedded Media entities, I spotted a small problem in the missing entity indicator that #2982322: Mark missing embedded entities in WYSIWYG introduced. It's using file_create_url(), but is not wrapping that in a call to file_url_transform_relative().

When not using file_url_transform_relative(), the Entity Embed filter would have to vary by the url.site cache context.

CommentFileSizeAuthor
#2 3063837-2.patch1.94 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
3063837-2.patch1.94 KB
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

When you don't control the output, such as with imagefieldformatter, it will output a full url, for example line 99 of /tests/src/Functional/ImageFieldFormatterTest.php

$this->assertSession()->linkByHrefExists(file_create_url($this->image->getFileUri()), 0, 'Link to the embedded image exists.');

http://my-local-test-site.local/sites/simpletest/15008825/files/image-test.png

I think in that case it's using a core template image.html.twig, if I remember correctly. So the entity embed filter would have to vary by the site.url for that, right?

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

No, it's up to the formatter that the Entity Embed filter calls to associate that cache context. The file URL being generated in this particular case is the only file URL being generated by the Entity Embed filter, so this is the only case where it has that responsibility.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs review » Reviewed & tested by the community

OK, this change looks good to me, RTBC.

  • Wim Leers committed 3f8edf5 on 8.x-1.x 
    Issue #3063837 by Wim Leers, oknate: Follow-up for #2982322: should use...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Reviewed & tested by the community » Fixed

🚢

Status: Fixed » Closed (fixed)

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