The MediaFormatResolver takes only the first media format of the MediaArgs into account when it comes to generating additional media formats for the responsive widths - see MediaFormatResolver.java#L171-L176
This forces the responsive source set to be of the first media format, even if the original rendition has another supported media format. Hence you cannot have a responsive image that supports multiple media formats.
An image component supports 16:9, 2:3 and 1:1 media formats. If you set the image sizes 100, 200 and 300, the media handler only renders 16:9 responsive images. Now if you declare all responsive widths as optional, you can get also 2:3 and 1:1 images but they are not responsive. Only 16:9 images are rendered with a source set.
Expected behavior: all valid media formats must be rendered as responsive images.
MediaFormatResolver should generate the additional media formats for all available media formats in the MediaArgs.
Any objection or thoughts on this? Otherwise I will prepare a PR
To solve the problem, the additional media formats (for responsive widths) should be generated for all media formats, not only for the primary one. But if we put them all in a flat list, the asset must fit to all combination of original media formats and the additional ones, which is very unlikely. We should rather put the additional media formats into a sublist of the original format. That means if the asset matches the original media format, it should also match the additional width formats related to the original one.
Example: suppose we have
mediaformats: mf1, mf2
widths: w1, w2, w3?
The rendition check must be as follows: the asset must match at least one of the following combinations:
[mf1, mf1_w1, mf1_w2, mf1_w3?]
[mf2, mf2_w1, mf2_w2, mf2_w3?]
I guess this requires refactoring of the MediaArgs class, so that instead of having one array of MediaFormatOptions, we have an array of arrays of MediaFormatOptions, or a map of original media format to additional ones <mf1: [mf1_w1, mf1_w2, mf1_w3]>, <mf2: […]>, …
An alternative solution is to put all media format combinations in a flat list in MediaArgs and only in MediaSource::resolveAllRenditions separate them in different lists, which are checked independently. This is a bit hacky because we should detect the related mediaFormats with a regexp like <string>__<digits> but has the advantage that the API of MediaArgs remains intact, which is good for backward compatibility.
what do you think? any better ideas?
I fix the problem as follows:
MediaFormatResolver generates additional width-based media formats for all original media formats, not only for the first one. These media formats have the property parentMediaFormat which contains the original media format.
MediaSource::resolveAllRenditions tries first to find renditions that match only the original media formats. Only when this was successful - that means any rendition is found and all mandatory renditions are resolved - it goes further and tries now to resolve the additional media formats of each of the original formats.
So the parent-child relationship is solved via mediaformat properties and we don’t need to touch the API and have no problem with backward compatibility.
Please note that the whole logic applies to image sizes only because picture sources have each exactly one mediaformat and its own widths. So they don’t need this parent-child relationship.
PR looks good to me! - i've only some rather cosmetic comments.