Request: Coerce VH `sizes` argument to `string` before calling `createImageTagWithSrcset`

[LeoniePhiline]

The sizes argument is defined in ImageViewHelper and MediaViewHelper as such:

        $this->registerArgument(
            'sizes',
            'string',
            'Sizes query for responsive image.',
            false,
            '(min-width: %1$dpx) %1$dpx, 100vw'
        );

However, arguments provided through fluid templates are not automatically coerced into the defined type (here: 'string').

This means that e.g. in MediaViewHelper::renderImageSrcset, this call might fail if sizes is not a string:

        // Generate image tag
        $this->tag = $this->responsiveImagesUtility->createImageTagWithSrcset(
            $image,
            $fallbackImage,
            $this->arguments['srcset'],
            $cropArea,
            $focusArea,
            $this->arguments['sizes'],
            $this->tag,
            false,
            $this->arguments['lazyload'],
            $this->arguments['ignoreFileExtensions'],
            (int) $this->arguments['placeholderSize'],
            $this->arguments['placeholderInline'],
            $fileExtension
        );

Here is a real life partial where exactly this happens:

  <f:if condition="{avatars.0} && {avatars.0.properties.height}">
    <figure>
      <f:comment><!--

      The images are displayed with a fixed height of 530px.
      Initially, all images have a height of 530px - this might change later when images are replaced.
      The initial images have various widths. This is of course problematic when responsive images
      do not deal with height, but only with width (which is nonsense in itself, but sadly standard).

      Scale factor:
        Actual image height / target image height (530px).

      The actual image width must be multiplied by that scale factor.

    --></f:comment>
      <f:variable name="width" value="{530 / avatars.0.properties.height * avatars.0.properties.width}" />
      <sms:media file="{avatars.0}"
                 sizes="{width}"
                 srcset="{width}, {width * 2}, {width * 3}"
                 lazyload="1" placeholderSize="20" placeholderInline="1"
                 alt="{avatars.0.alternative}" title="{avatars.0.title}"
                 class="" />
      <f:if condition="{avatars.0.description}">
        <figcaption class="hidden">
          {avatars.0.description -> f:format.nl2br()}
        </figcaption>
      </f:if>
    </figure>
  </f:if>

As width is a variable assigned by evaluating a math expression, it is of type int when it is fed into the MediaViewHelper.

This hard-fails with:

(1/1) TypeError

Sitegeist\ResponsiveImages\Utility\ResponsiveImagesUtility::createImageTagWithSrcset():
Argument #6 ($sizesQuery) must be of type ?string, int given, 
called in /var/www/html/public/typo3conf/ext/sms_responsive_images/Classes/ViewHelpers/MediaViewHelper.php
on line 268

I propose coercing types manually, since TYPO3 (11) does not do that, despite arguments being declared with types:

        // Generate image tag
        $this->tag = $this->responsiveImagesUtility->createImageTagWithSrcset(
            $image,
            $fallbackImage,
            $this->arguments['srcset'],
            $cropArea,
            $focusArea,
            (string $this->arguments['sizes'],
            $this->tag,
            false,
            $this->arguments['lazyload'],
            $this->arguments['ignoreFileExtensions'],
            (int) $this->arguments['placeholderSize'],
            $this->arguments['placeholderInline'],
            $fileExtension
        );

Here, only $this->arguments['sizes'] is force-cast to string (and, unchanged, placeholderSize to int).

This might be recommendable for all incoming $this->arguments where receiving functions have strict argument type restrictions.

See also https://github.com/sitegeist/sms-responsive-images/commit/f13d0cbd3080d65eebf8250de9b0d6d8473fbecf

[s2b]

Thank you for your PR! I agree that this is unwanted behavior, however I don't think that sizes="123" is valid HTML. I think that it is required to specify a CSS unit there, for example sizes="123px".

https://html.spec.whatwg.org/multipage/images.html#sizes-attributes

[LeoniePhiline]

Holy cow, how haven’t I noticed that? I just checked the file history, and this bug has been in the template for several years. Mind blowing.