- Issue created by @alxgl
- ๐ฏ๐ดJordan Rajab Natshah Jordan
Rajab Natshah โ made their first commit to this issueโs fork.
- Merge request !14Issue #3456085: Fix Drupal 10.3 compatibility for Too few arguments to function deliver() 3 passed in Drimage โ (Open) created by Rajab Natshah
- Status changed to Needs review
6 months ago 10:52pm 20 June 2024 - ๐ฏ๐ดJordan Rajab Natshah Jordan
Attache a static patch file for drimage 2024-06-21 MR 14
To be used with composer patches - ๐ฏ๐ดJordan Rajab Natshah Jordan
Thanks, Alexandre, for reporting and hinting.
You gave a quick hit on how to fix this issue.
Sorry, if you had an attend to provide an MR or a Patch.
Needed to have a quick fix for this issue in our product.Please, review, still we may need to check if in some cases,
The fix may need to provide a'private'
not to have'public'
all the time. - ๐ซ๐ทFrance alxgl
Thanks to you Rajab, for this quick patch. I'll write a feedback asap.
- Status changed to RTBC
6 months ago 8:02am 21 June 2024 - ๐ฎ๐นItaly finex
Hi, thank you for the patch. It works correctly. Tested on three websites without any issue.
- ๐ซ๐ทFrance alxgl
I don't have the time right now to give a better proposition, but I think we should be closer than what have been proposed here for stage_file_proxy, in terms of coding standard : https://git.drupalcode.org/project/stage_file_proxy/-/commit/a26b05f77ad...
- Status changed to Needs review
6 months ago 7:50pm 21 June 2024 - ๐ฏ๐ดJordan Rajab Natshah Jordan
Thanks, Leonardo and Alexandre, for following up.
I had 2nd round of testing reviewing the code in both modules.
The Stag File Proxy module has extended the ImageStyleDownloadController class, with custom override implementation for the
deliver
method.
they had to update that.The Drimage module has extended the ImageStyleDownloadController class but did not override the
deliver
method.
It is only calling it.My only concerns at this time are:
- When to pass
'private'
and when to pass'public'
? - Is using
"public"
all the time is the optimal logic? - Should we add a new override implementation for the
deliver
method to allow for public or private cases?
- When to pass
- Status changed to Needs work
6 months ago 6:26am 26 June 2024 - ๐ง๐ชBelgium weseze
I think the public/private detection is already done somewhere in the image() function in the controller. We should be able to pass it correctly.
Quick question, since I'm still running 10.2 atm, this change would make it break with 10.2?Will have a look at this at the end of the week and try to get it release.
- Status changed to Needs review
5 months ago 9:48am 1 August 2024 - ๐ฏ๐ดJordan Qusai Taha Amman
We should use $scheme variable as in this way will be more dynamic as I have a case the scheme will be s3 so it needs to be dynamic
- ๐ง๐ชBelgium lorenzs
Tested the latest patch from #13 and works well.
This is way better than putting it hardcoded to 'public'. Only small drawback I see is that the derivative image will always have the same scheme as the original image (where the scheme is derived from the uri).
So I think only very specific use cases where sites are putting derivatives and originals in different locations (f.e. public & s3 if no general takeover of public) will encounter issues with this but all others can work with this already. - Status changed to RTBC
3 months ago 1:03pm 2 October 2024 -
weseze โ
committed ea89ea03 on 2.x
Issue #3456085 by rajab natshah, qusai taha, weseze: Fix Drupal 10.3...
-
weseze โ
committed ea89ea03 on 2.x
Automatically closed - issue fixed for 2 weeks with no activity.