sitegeist / sms-responsive-images

This TYPO3 extension provides ViewHelpers and configuration to render valid responsive images based on TYPO3's image cropping tool.
GNU General Public License v2.0
34 stars 18 forks source link

Wrong encoding of crop-attribute's array #98

Closed sir-gawain closed 1 year ago

sir-gawain commented 1 year ago

If an own crop configuration is given to crop="" attribute, the core returns it as array in line 127 of ImageViewHelper.php. This array is then converted to string by a cast, and given to create():

$cropVariantCollection = CropVariantCollection::create((string)$cropString);

But this create() expects an json encoded string (at least in TYPO3 11) to there will occure an error in decoding.

This line must be changed to

$cropVariantCollection = CropVariantCollection::create(is_string($cropString) ? $cropString : json_encode($cropString));

s2b commented 1 year ago

Is this an issue that also exists in TYPO3 core by any chance? From my point of view, the code used in the core and in sms-responsive-images is the same:

Core: https://github.com/TYPO3/typo3/blob/11.5/typo3/sysext/fluid/Classes/ViewHelpers/ImageViewHelper.php#L157 Extension: https://github.com/sitegeist/sms-responsive-images/blob/master/Classes/ViewHelpers/ImageViewHelper.php#L127

The definition in the core is in fact a string or boolean, not an array: https://github.com/TYPO3/typo3/blob/11.5/typo3/sysext/fluid/Classes/ViewHelpers/ImageViewHelper.php#L123

So if you want to use the attribute, you have to provide JSON to the ViewHelper:

(example taken from #99)

<f:variable name="croppingConfiguration" value="{listview:{cropArea:{x:0,y:0,width:1,height:1},allowedAspectRatios:{vorschau:{title: "vorschau", value: 1.48}},selectedRatio:"vorschau"}}" />
<f:image image="{myImage}" crop="{croppingConfiguration -> f:format.json()}" />
s2b commented 1 year ago

Please let me know if this issue is still relevant

sir-gawain commented 1 year ago

Yes, the issue can be resolved by using your solution with f:format.jsonencode(). Forgot to reply earlier. Perhaps an notice in the documentation would be helpful to others, as the usage of this parameter is not self documenting.