-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow <source> to use <img>'s sizes data as a fallback #6633
Comments
Here's a worked example: <p>Currently, need to duplicate sizes:</p>
<picture>
<source type="image/avif"
media="(min-width: 800px)"
srcset="f1-689.avif 689w,
f1-1378.avif 1378w,
f1-500.avif 500w,
f1-1000.avif 1000w"
sizes="(min-width: 1066px) 689px,
calc(75vw - 137px)">
<source media="(min-width: 800px)"
srcset="f1-689.jpg 689w,
f1-1378.jpg 1378w,
f1-500.jpg 500w,
f1-1000.jpg 1000w"
sizes="(min-width: 1066px) 689px,
calc(75vw - 137px)">
<source type="image/avif"
srcset="f1-focused-800.avif 800w,
f1-focused-1406.avif 1406w"
sizes="(min-width: 530px) calc(100vw - 96px),
100vw">
<img alt="F1 car"
src="f1-cropped-800.jpg"
srcset="f1-focused-800.jpg 800w,
f1-focused-1406.jpg 1406w"
sizes="(min-width: 530px) calc(100vw - 96px),
100vw">
</picture> In this case I've split the However, it feels like the way I've split the With my proposal, it would be: <p>No duplicate sizes!</p>
<picture>
<source type="image/avif"
media="(min-width: 800px)"
srcset="f1-689.avif 689w,
f1-1378.avif 1378w,
f1-500.avif 500w,
f1-1000.avif 1000w">
<source media="(min-width: 800px)"
srcset="f1-689.jpg 689w,
f1-1378.jpg 1378w,
f1-500.jpg 500w,
f1-1000.jpg 1000w">
<source type="image/avif"
srcset="f1-focused-800.avif 800w,
f1-focused-1406.avif 1406w">
<img alt="F1 car"
src="f1-cropped-800.jpg"
srcset="f1-focused-800.jpg 800w,
f1-focused-1406.jpg 1406w"
sizes="(min-width: 1066px) 689px,
(min-width: 800px) calc(75vw - 137px),
(min-width: 530px) calc(100vw - 96px),
100vw">
</picture> Now there's no duplication, and all the layout information is in one place. |
Alternatively, rather than falling back to That means you can still split <p>No duplicate sizes!</p>
<picture>
<!-- this gets its sizes from the next <source>
since the media also matches -->
<source type="image/avif"
media="(min-width: 800px)"
srcset="f1-689.avif 689w,
f1-1378.avif 1378w,
f1-500.avif 500w,
f1-1000.avif 1000w">
<source media="(min-width: 800px)"
srcset="f1-689.jpg 689w,
f1-1378.jpg 1378w,
f1-500.jpg 500w,
f1-1000.jpg 1000w"
sizes="(min-width: 1066px) 689px,
calc(75vw - 137px)">
<!-- this gets its sizes from the <img> -->
<source type="image/avif"
srcset="f1-focused-800.avif 800w,
f1-focused-1406.avif 1406w">
<img alt="F1 car"
src="f1-cropped-800.jpg"
srcset="f1-focused-800.jpg 800w,
f1-focused-1406.jpg 1406w"
sizes="(min-width: 530px) calc(100vw - 96px),
100vw">
</picture> |
It seems likely that it will be the same value for every The main crux I guess is that A very early design of That said, it seems not terrible to fall back to the |
Yeah, that seems fair to me |
Why not allow |
@Yay295 that would be inconsistent with all other attributes involved in picture/source/img. The img is what owns the resource selection, and all "fallback" attributes (src, width, height, alt...) are on the img. |
What would the effect be on cases where |
Right now, it'll take a default of The change will cause compatibility problems with sites that have deliberately omitted the |
What I am seeing in our sites ( tiny sample size, but still :) ) is that
There is a very small group of cases that are wrong and will need updating, but personally I am fine with that. It does not help that there is no code snippet on MDN showing The docs do clearly state the purpose and necessity of the |
@jakearchibald are you interested in investigating the web compat impact? An elaborate HTTP Archive query (or series of queries) could maybe work, or implementing a use counter that triggers when the selected Even for cases that do match that, for it to be "breaking" (in terms of changed layout, not just which image was loaded from srcset), it also needs to have no dimensions specified with CSS or the |
Yeah, I can take this on, and I agree with your analysis. |
@zcorpan here's my plan, let me know if it sounds bad (I don't have a ton of HTTP Archive experience):
The selected image changing is ok, but it'd be interesting to know. The layout changing would be a worry. |
I ran a query today to capture all (I tried sharing the resulting table with you, but got an error. Oh well.)
This could probably be checked in step 1 before running matches through the puppeteer script, to reduce the number of pages to run through step 2. (I figure if you're running step 2 on your own computer, you want a not-so-huge dataset so it doesn't take forever to complete.) But yeah, I think that should find the interesting cases! |
It still needs to be checked in puppeteer in case the source has changed, or the RegEx got it wrong. |
@zcorpan here's the query I'm using to gather the URLs. Let me know if something looks amiss: SELECT
page
FROM (
SELECT
page,
ARRAY_TO_STRING(REGEXP_EXTRACT_ALL(LOWER(body), r'<source[^>]*\ssrcset=[\'"][^\'"]*\s\d+w[^\'"]*[^>]*>'), '\n') AS markup
FROM
`httparchive.latest.response_bodies_desktop`
WHERE
page = url )
WHERE
markup != ''
AND NOT REGEXP_CONTAINS(markup, r'sizes=') |
@jakearchibald it looks OK. There are some minor tweaks to make it a bit more correct (like allow spaces around the |
Phase 1: From 5,642,228 HTML pages, 3251 (0.06%) matched the regex in the above query. Phase 2: Of that group, 258 were found to have a Phase 2 was done with node & JSDOM, so pages may have changed since the versions in the HTTP archive. 123 of the URLs from page 1 failed to fetch. Numbers here seem so small, I don't think it's worth digging in further. WDYT @zcorpan?
One-liner to find impacted [...new Set($$('source[srcset]:not([sizes])').map(el => el.closest('picture')))].filter(el => { const img = el.querySelector('img'); return img && img.sizes && img.sizes !== '100vw';}) |
Picking a few at random: https://www.harpercollinschristian.com/ - the picture elements here are really weird. The width descriptor is very different to the actual image widths. https://waterlesshaircare.com/en-us - the sources only have one item in the https://www.dominatoryachts.com/en - the proposed change would improve src selection on this page. https://www.yalehome.co.nz/en/ - this is an interesting one: <picture>
<source
srcset="/presets/small-mobile/Yale/yalehomeCONZ/Banners/store-old%20mobile.jpg 480w, /presets/medium-mobile/Yale/yalehomeCONZ/Banners/store-old%20mobile.jpg 767w"
media="(max-width: 767px)">
<img
src="/presets/large-desktop/Yale/yalehomeCONZ/Banners/store-old.jpg"
srcset="/presets/small-desktop/Yale/yalehomeCONZ/Banners/store-old.jpg 768w, /presets/medium-desktop/Yale/yalehomeCONZ/Banners/store-old.jpg 1345w, /presets/large-desktop/Yale/yalehomeCONZ/Banners/store-old.jpg 1800w"
sizes="(min-width: 767px) 100vw, 1800px">
</picture> If I'm reading that right, the change proposed here would result in browsers selecting a higher resolution image than needed at smaller (<767px) widths. https://springandvine.com/en-us - another one with a single item in each srcset. The https://www.allianz.co.uk/ - the sizes are pretty wrong here, but it looks like the change proposed here would improve image selection. |
Hehe, we make up 2% of affected sites. |
@romainmenke hah, this gives me some confidence that the test is correct |
PR #6695 |
@zcorpan do you know who works on this stuff at Safari & Firefox, to see if they're happy with this change? |
258 out of 5,642,228 is ~0.0046%, and per #6633 (comment) those cases are mostly using |
Echoing the comments, the use of sizes is nearly mostly wrong / weird. In the 2019 almanac we found the use of sizes in the proper way was very limited. Specifically, the patterns we found boiled down to:
Of which the last style was pretty rare. |
The last one is 'most correct' right? (although I personally use min-width, as that's how I write my media queries) |
By which "most correct" I would qualify it that it appears that a human hand crafted an intention and was generally unique across sites. The example snippet above was a random selection for illustration. I'll see if I have the results of the original query still handy (see: 04-06.sql and 04-06b.sql ). |
@colinbendell: That's a nice wrap-up of
|
Within a
<picture>
,<source>
and<img>
can have asizes
attribute.sizes
defines the layout width of the image, in CSS pixels. It supports media conditions so you can provide different layout widths for different viewport widths.Currently, this must be specified on every
<source>
and<img>
that usesw
units in itssrcset
. However, it feels like thesizes
data will frequently be the same in every<source>
and<img>
within a given<picture>
, since the image's width is dictated by the CSS that operates at a document level.To avoid duplication, should
<source>
take its sizes data from the<img>
if it doesn't have asizes
attribute of its own?@zcorpan, I'm told you might have thoughts here.
The text was updated successfully, but these errors were encountered: