Skip to content
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

Should Sec-CH-Width be sent when the sizes attribute is not present? #19

Closed
eeeps opened this issue Aug 3, 2021 · 9 comments · Fixed by #27
Closed

Should Sec-CH-Width be sent when the sizes attribute is not present? #19

eeeps opened this issue Aug 3, 2021 · 9 comments · Fixed by #27

Comments

@eeeps
Copy link
Collaborator

eeeps commented Aug 3, 2021

We state that:

For fetches trigged by img elements, its value SHOULD be calculated by multiplying the source set’s current source size by the Window's current devicePixelRatio.

Some facts:

Should Chrome change, or should the spec?

@eeeps
Copy link
Collaborator Author

eeeps commented Aug 4, 2021

Right now the use case for Sec-CH-Viewport-Width in the spec example is as a fallback for Sec-CH-Width, when it is not present.

The most common reason Sec-CH-Viewport-Width is missing, is that the <img> does not have a sizes attribute.

If Sec-CH-Width sent 100vw * devicePixelRatio in cases where there is no sizes on the img (per spec, at the moment), instead of no hint at all (Chrome's implementation, at the moment):

Pros

  • Consistency with srcset-based selection flow
  • Simpler server-side implementation story
  • The most common use case for RI-CH no longer needs Sec-CH-Viewport-Width
    • UAs can send a few less bits of (often unused) information about themselves; good for privacy
    • Varying gets simpler (I think)

Cons

  • Servers are unable to tell the difference between cases where sizes is present, and not. I recently sketched out a fallback flow that gave Width more priority than anything, but author-supplied fallbacks more priority than Viewport-Width. Such a flow would be impossible, with this change.

On first read, the pros outweigh the cons; I'll file a Blink issue.

@yoavweiss
Copy link
Collaborator

  • Servers are unable to tell the difference between cases where sizes is present, and not. I recently sketched out a fallback flow that gave Width more priority than anything, but author-supplied fallbacks more priority than Viewport-Width. Such a flow would be impossible, with this change.

I'm concerned about this. At the same time, I think that if we're reconsidering this, it makes sense to also reconsider setting the right width value in cases where the browser knows it (e.g. for lazy loaded images, late loaded images that are layed out in containers that don't depend on their intrinsic size, etc).

@eeeps
Copy link
Collaborator Author

eeeps commented Aug 31, 2021

@yoavweiss You're suggesting tying this work to progress on whatwg/html#4654 ?

In a perfect world, that issue will be fixed by some mechanism that updates the "source set’s current source size," and the language here will end up just as it is now.

In the meantime, do you think it's a good idea to update the language here to make the sending of Sec-CH-Width contingent on the presence of the sizes attribute, to match Chrome?

In general, I'm leaning towards models that are easier to understand/predict for authors, over models that offer servers maximum knowledge and flexibility. As such, I moderately prefer making this work just like srcset/sizes. But I could be convinced.

@yoavweiss
Copy link
Collaborator

@yoavweiss You're suggesting tying this work to progress on whatwg/html#4654 ?

I am suggesting that. I think that sizes is clearly not needed in those cases, and we should not ask developers to add it. I agree this is a broader subject than just Sec-CH-Width (as it would also have implications on srcset's source selection).

In general, I'm leaning towards models that are easier to understand/predict for authors

I think the model is easy to understand: when loading=lazy is present, sizes is automatically calculated.

I moderately prefer making this work just like srcset/sizes

I agree and think there's no contradiction here. We need to automatically calculate sizes for both srcset and CH.

@eeeps
Copy link
Collaborator Author

eeeps commented Sep 7, 2021

In general, I'm leaning towards models that are easier to understand/predict for authors

I think the model is easy to understand: when loading=lazy is present, sizes is automatically calculated.

Oh, totally agree. I think what's confusing is that Sec-CH-Width doesn't have the same 100vw fallback that sizes does.

I think we're largely on the same page, as far as where the puck is heading. I think this spec is already where it needs to be:

Right now, per HTML:

  • Every <img> (whether it has a sizes or not) gets a source size before it generates a request.
  • In cases where there is no sizes attribute, the default source size of 100vw is used.

And per this spec:

  • That source size should be used to generate Sec-CH-Width.

Source size calculation (in HTML) can get smarter, without affecting this spec. The issue at hand is that Chrome's implementation doesn't send Sec-CH-Width at all if the sizes attribute is missing. At some point, either this spec or Chrome's implementation will have to change so that they agree. My sense of where we're at is:

  • I think that Chrome should change to match the spec (send 100vw [in px] when there's no sizes) now. I think this change would make RI-CH less confusing for authors, and then if source size calculation gets smarter later, we get those benefits automatically.
  • You think that neither Chrome nor this spec should change until the particulars of smarter source size calculation are figured out in Add "auto sizes" for lazy-loaded images whatwg/html#4654.

Accurate?

@yoavweiss
Copy link
Collaborator

Yeah, that's accurate. The reason Chromium doesn't currently send Width when sizes are not defined is to enable server developers to distinguish between cases where it is and isn't (and presumably fall back to Viewport-Width in the latter case).

If this reasoning is flawed, than we could move on one aspect of the problem before moving on the other. (even if I'd prefer to look at it holistically)

@jonarnes
Copy link

jonarnes commented Sep 9, 2021

In general I think it's a good idea for the user-agent to send Width whenever the value is known. Not only from sizes but from lazy loading scenarios, as well as other cases where the width of an image is explicitly defined by the developer. For example when the good old width attribute is present. FWIW, I see devs "retrofitting" server side image optimisation by simply doing like this: <img src="" width="200" sizes="200px"/>

My opinion is that Width should always provide the exact intrinsic pixel size needed by the UA. Just like when sizes is set today. Mixing the viewport size in here would make `width´ have multiple meanings (overloaded).

The Con outlined above is a very big con, imo. A UA with a high DPR would then request a huge image from image cdns (at least that's how ImageEngine works). A trend I observe is that DPR is "capped" at some level, simply because a 4x image does not have additional value compared to the 2x variant. So the ability for devs, or image cdns, to make decisions based on variables (hints) with a single meaning is crucial.

@eeeps
Copy link
Collaborator Author

eeeps commented Jun 2, 2023

So given:

  1. how Add "auto sizes" for lazy-loaded images whatwg/html#4654 turned out
  2. the desire to disambiguate cases where we got a fallback sizes, and when we didn't

I did my best. Reviews welcome!

If we agree on this approach, I can whip up a Web Platform Test and try to make sure Chrome's in-progress implementation passes before it ships.

@eeeps
Copy link
Collaborator Author

eeeps commented Jun 2, 2023

@jonarnes

Not only from sizes but from lazy loading scenarios...

whatwg/html#4654 means that lazyload scenarios get automatic sizes, so we can continue to read from the parsed sizes value, yay.

For example when the good old width attribute is present

Hopefully this represents the largest-possible intrinsic size of the image, and (more importantly) combines with height to give the intrinsic aspect ratio. These attributes are not meant to style the image, in responsive contexts.

Mixing the viewport size in here would make `width´ have multiple meanings (overloaded).

I get that. What's cleaner from a spec perspective isn't always easier for developers using the spec to understand. How does the PR look to you?

@eeeps eeeps closed this as completed in #27 Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants