- Issue created by @Wouter Waeytens
- Status changed to Needs work
7 months ago 12:21pm 15 May 2024 - last update
7 months ago Custom Commands Failed - Status changed to RTBC
7 months ago 12:21pm 17 May 2024 - ๐ต๐ฐPakistan isalmanhaider
+1
#2 worked! Tested on Drupal 11.0-dev, PHP 8.3.
Good catch and a must-have fix.
By default, it pulls low resolution (480 ร 360 pixels) and stores as original style.
After applying the patch, it pulls high resolution (1280 ร 720 pixels) and stores as original style. - Status changed to Needs work
7 months ago 1:43pm 17 May 2024 There is a patch here that failed testing. Please fix that and merge requests are preferred.
Also: It isn't clear what the screenshots in comment #6 are representing as supporting this issue being ready to commit.
- ๐ณ๐ฟNew Zealand quietone
Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.
- ๐ฎ๐นItaly bessone
The patch also worked on my Drupal 10.3.2 (with Thunder distro).
I haven't done any testing, just added the patch in composer.json on a demo I'm working on.
- ๐ฎ๐ณIndia Sahana _N
sahana _n โ made their first commit to this issueโs fork.
- Status changed to Needs review
3 months ago 1:38pm 17 September 2024 - ๐ฎ๐ณIndia Sahana _N
Hi,
I created an MR Please review I tested the test cases, and now the test cases are working fine.
I would greatly appreciate your suggestions for improvement. Please let me know.
Thank you!!
- Status changed to Needs work
3 months ago 1:46pm 17 September 2024 - Status changed to Needs review
3 months ago 8:30am 20 September 2024 - ๐ฎ๐ณIndia Sahana _N
Hi @smustgrave ,
Thanks for the review.
I updated the MR. The test coverage for the provider's name is already covered.
Path: core/modules/media/tests/src/Functional/ResourceFetcherTest.phpPlease let me know for other concerns. I am happy to take suggestions.
Thank you!!
- Status changed to Needs work
3 months ago 1:36pm 20 September 2024 - ๐บ๐ธUnited States smustgrave
Test coverage should be expanded for additional changes.
- First commit to issue fork.
- ๐ฉ๐ชGermany daveiano
While it works and gets the
maxresdefault
image for most YouTube Videos, it does not work every time. There is a chance that a video does not provide amaxresdefault
image, just the defaulthqdefault
. I guess that's something to do with the video size.I added a check that the
maxresdefault
image is only used, when available. Otherwise, it will just return a 404.Before the change, this would leave media entities without a thumbnail at all.
- ๐ซ๐ทFrance eliechoufani Marseille, Beirut
Hello,
I had the same problรจme on Drupal 10.3.5
Trying the patch in #2 didn't solve the problem because the http 404 error request was overriding the response.
So I did this patch that's working for my use case, maybe it needs more testing
- ๐ง๐ชBelgium Wouter Waeytens
I can confirm that patch in #19 works :)
- ๐บ๐ธUnited States recrit
There are a few issues with the patch in #19:
- In the new method doVimeoRequest, the TransferException is not defined and needs to be imported.
- the import "use GuzzleHttp\Client" is never used.
- The fallback for Youtube should be uncommented:
catch (RequestException $e) { // Use default thumbnail. //$data['thumbnail_url'] = str_replace('maxresdefault', 'hqdefault', $data['thumbnail_url']); }
- ๐บ๐ธUnited States recrit
Pushed some fixes for better error handling. This is now working for me when Vimeo does not have a high resolution option.
Thinking about this more, I feel like it could be better handled in\Drupal\media\Plugin\media\Source\OEmbed::getLocalThumbnailUri()
so that the high resolution image is only requested once and then the local file is created. Currently, the fixes in this issue would causes the high resolution images to be request every time the resource cache is rebuilt. This seems a bit excessive to me.