envygeeks / jekyll-assets

:art: Asset pipelines for Jekyll.
ISC License
1.11k stars 169 forks source link

Responsive images: <img> tag must be after all <source> tags #433

Closed migueldemoura closed 6 years ago

migueldemoura commented 7 years ago

Regarding responsive images, the <img> tag must be after all <source> tags

Current version outputs the following:

<picture>
    <img src="default.png">
        <source media="(min-width: 480px)" srcset="medium.png">
        <source media="(min-width: 0px)" srcset="small.png">
</picture>

which should be:

<picture>
        <source media="(min-width: 480px)" srcset="medium.png">
        <source media="(min-width: 0px)" srcset="small.png">
        <img src="default.png">
</picture>

otherwise the default.png will always be loaded.

OS: Ubuntu 17.10 Jekyll: 3.6.2 Jekyll-Assets: current master - https://github.com/jekyll/jekyll-assets/commit/4db146d24ddcec144738e127671314c84855708e

Edit: change the order here? https://github.com/jekyll/jekyll-assets/blob/61fa0aebd97c1f6bdcfe24ab7fa07ea28c6df667/lib/jekyll/assets/plugins/html/pic.rb#L34-L38

envygeeks commented 7 years ago

There is no need to link me to my own source, I wrote it, I know how and where things happen.

envygeeks commented 7 years ago

Please see: https://github.com/jekyll/jekyll-assets/issues/421#issuecomment-342266836 it is confirmed that this is not the case, and it is backed up by one of the people who helped write the HTML spec.

At this time I feel this is not a bug, but rather an arbitrary preference of which I do not plan to change.

migueldemoura commented 7 years ago

Not doubting the spec, but the implementation on Chromium 62 and Firefox 58 requires the img tag to be at the end:

<picture>
    <source media="(min-width: 480px)" srcset="https://dummyimage.com/1200x1200/000/fff&text=480px">
        <source media="(min-width: 0px)" srcset="https://dummyimage.com/480x480/000/fff&text=0px">
    <img src="https://dummyimage.com/1200x1200/000/fff&text=default">
</picture>

<picture>
    <img src="https://dummyimage.com/1200x1200/000/fff&text=default">
    <source media="(min-width: 480px)" srcset="https://dummyimage.com/1200x1200/000/fff&text=480px">
        <source media="(min-width: 0px)" srcset="https://dummyimage.com/480x480/000/fff&text=0px">
</picture>

Opening that .html at, for example, 1200px width will display "480px" on the first image and "default" on the second one.

chromium ff

Could you please verify this behaviour?

envygeeks commented 6 years ago

That does seem weird, can you explain this @nhoizey?

envygeeks commented 6 years ago

Since this is indeed the case from my own simple testing I'm gonna have to fix this, lest we face a thousand bugs over it, but it does seem odd.

migueldemoura commented 6 years ago

I glanced over the spec and it seems that all examples use the img tag at the bottom, which may explain why these browsers implemented it that way. Couldn't find a requirement for it to be so, though.

nhoizey commented 6 years ago

@envygeeks as @migueldemoura said, I think there is no requirement in the spec to put the <img> at the end of <picture>, but all examples do that, so maybe browsers vendors considered this as a requirement.

I seems indeed safer to follow this.

Maybe @grigs or @yoavweiss can help understand this.

envygeeks commented 6 years ago

Indeed, I thought there was no requirement on order too, I also agree that if vendors want it last then we should probably enforce that so that users have no problems, but it would be interesting to find out why. I'll try to get hold of a friend on Chrome team and see if there is any particular reason too, but lets just go with the assumption that I will be making sure <img> comes last if that's what vendors want, regardless.

Normally I would write this off if it's just one vendor being an odd ball, but that's clearly not the case.

yoavweiss commented 6 years ago

The <img> tag is part of the source selection algorithm, which walks through the potential image sources and picks to first one that fits. since <img> is always a good fit, putting it first means that the <source> tags after it will get ignored.

envygeeks commented 6 years ago

That does make sense, thanks @yoavweiss. The same reason we have to order our examples a specific way so that users can copy and paste because of the matching and indeed, <img> would be the always matched.

nhoizey commented 6 years ago

Thanks @yoavweiss for the clarification.

I should read the spec once again. While always putting the <img> at the end, I always assumed it was not required.