Problem/Motivation
When downloading the thumbnail for an oEmbed resource, the media system needs to figure out what the thumbnail's file extension will be, so that it (the thumbnail) can be used by the image system.
Most of the time, the file extension is right there in the thumbnail URL. But other times, the thumbnail URL has no file extension. In such a case, we make a HEAD request to the thumbnail URL and try to use the Content-Type header it gives us, if any, to determine a file extension. That, however, has two drawbacks:
- Downloading the thumbnail can result in two HTTP requests (one HEAD and one GET), rather than just one.
- Some providers, like TikTok, block HEAD requests. This results in the downloaded thumbnail having no file extension, and therefore being unusable by the image system.
Steps to reproduce
You'll need custom code, but create a media source plugin that extends the oEmbed source, and supports TikTok as a provider. You'll see that any media you add with that source will not get a proper thumbnail, since the thumbnail URLs don't have a file extension, and TikTok blocks HEAD requests. This was revealed by #3225184: oEmbed system cannot retrieve thumbnail extensions from TikTok / Vimeo.
(A test module was written for this; see #2.)
Proposed resolution
I think we can kill both of the aforementioned birds with one stone.
If we can't tease the thumbnail file extension out from the thumbnail URL, let's examine the Content-Type header in the GET response (that actually downloads the thumbnail) and derive a file extension that way. Not only will this be more likely to succeed, but it completely removes any need to make a HEAD request.
Remaining tasks
Implement it with test coverage.
User interface changes
None.
API changes
TBD, but an existing protected method may receive a new parameter and/or a deprecation notice.
Data model changes
None.
Release notes snippet
TBD
Issue fork drupal-3231731
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedJust to add If any one want to test the issue, Here is the Module Which extends oEmbed Class. I have created a sandbox project for easier testing.
Steps to test :
1.Enable the Module.
2.Create Media entity by navigating to Content > Media > Add Media > TikTok . You can see the Thumbnail is being Rendered.
3. Now attach the TikTok Media to any Node Bundle and try to embed the TikTok media entity (New or already created one). You will observe that Media item is attached and rendered, but it wont display thumbnail.
Comment #3
phenaproximaComment #4
AnybodySame problem for vimeo as I posted in the other issue (#3225184):
Perhaps we should close one of them as duplicate?
As result no Vimeo Thumbails are created (for public videos) while it works for YouTube.
Comment #6
phenaproximaTook a shot at implementing the solution outlined in the issue summary.
Comment #7
phenaproximaSince this change would reduce the need to make an extra HTTP request, it can be considered a performance improvement. Tagging accordingly.
Comment #8
larowlanLooking nice, could only find a fault in the deprecation message
Comment #9
phenaproximaChange record written. I've updated the code accordingly and, to be sure, grepped contrib for
getThumbnailFileExtensionFromUrl
, which turned up no results (see http://grep.xnddx.ru/search?text=getThumbnailFileExtensionFromUrl&filename=).Comment #10
seanBThanks for the quick change @phenaproxima, I think this should cover it. Nice improvement!
RTBC assuming the test will be green.
Comment #11
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedHi @phenaproxima Thank you for Quick change. I tried to test this fix. I tested it on Drupal core 9.2.6 with patch applied. I still see the same issue that was present earlier for TikTok.
TikTok Video Thumbnails are still not being rendered similar to that what its present earlier. But it worked for Vimeo without any issues in my testing.
Comment #12
phenaproximaAh, sorry to hear it. However, since TikTok is not officially supported in core and this change is a performance improvement, I think we should commit it as-is. If you contact me in Slack, I'd happy to help figure out why it's failing for TikTok.
Comment #13
alexpottThis looks like a really nice improvement. Less requests is nice.
Committed 8597def and pushed to 9.3.x. Thanks!