Swap out custom code with image_style_url function Issue #6860 #5026
Conversation
There is a bunch of custom code in the render function of views_handler_field_file_uri.inc that attempts to generate the image style url. This code breaks when you're using modules such as s3fs which doesn't use the site default file path. I'm swapping that code out instead with image_style_url which fixes the problem.
|
Related to: backdrop/backdrop-issues#6860 |
|
Tugboat has finished building a preview for this pull request! Website: https://pr5026-lnljnm1fwceyeh9z9ju7gjnld6g41f82.tugboatqa.com/ This preview will automatically expire on the 27th of April, 2025. |
| $data = substr_replace($data, $add_to_url, $insert_at + $file_public_path_strlen, 0); | ||
| $url = file_create_url($data); | ||
| if (!image_is_svg($url) && !empty($this->options['image_style'])) { | ||
| $url = image_style_url($this->options['image_style'], $data); |
There was a problem hiding this comment.
In the (unlikely case) that the image module's off, this will fail with "Call to undefined function...". At least, that's what I assume.
The file module's a required one, but the image module isn't.
There was a problem hiding this comment.
The file already requires the image module to be on because for the options form it calls image_style_options()
I can add conditionals on all that and then in the render section check to see if the module is enabled, write a message to watchdog if it's not and return just the file path.
- Does that sound like the proper approach?
- Do I need something for the automated tests to test both cases with the image module on and off?
There was a problem hiding this comment.
The file already requires the image module to be on because for the options form...
That's a good point. How would we get a setting for $this->options['image_style'], anyway, without the image module available. So my first impression, that this odd code exists to avoid image module dependency seems wrong. A check would make more sense in function options_form().
Do I need something for the automated tests to test both cases with the image module on and off?
It's an edge case, I guess, but if you're willing to dig into that, it would be awesome. No requirement, though, as it's unrelated to the broken thumbnail problem. You decide. 😉
There is a bunch of custom code in the render function of views_handler_field_file_uri.inc that attempts to generate the image style url. This code breaks when you're using modules such as s3fs which doesn't use the site default file path. I'm swapping that code out instead with image_style_url which fixes the problem.
Fixes Issue 6860