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

Use width and height as intrinsic aspect ratio for img elements #4952

Merged
merged 5 commits into from
Oct 2, 2019

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Sep 30, 2019

When the current request is not available, or it doesn't have an intrinsic
ratio (such as some SVG content).

This specifies the proposal in
WICG/intrinsicsize-attribute#16, which is implemented
in both Firefox and Chromium behind a flag.

Tests for this change are at:

https://github.com/web-platform-tests/wpt/blob/a57ec1432f22ac42e8e219a32e2abd7c0baa5b09/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.tentative.html

(See WHATWG Working Mode: Changes for more details.)


/infrastructure.html ( diff )
/rendering.html ( diff )

When the current request is not available, or it doesn't have an intrinsic
ratio (such as some SVG content).

This specifies the proposal in
WICG/intrinsicsize-attribute#16, which is implemented
in both Firefox and Chromium behind a flag.

Tests for this change are at:

https://github.com/web-platform-tests/wpt/blob/a57ec1432f22ac42e8e219a32e2abd7c0baa5b09/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.tentative.html
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I like this second take modulo a couple nits.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@annevk annevk added addition/proposal New features or enhancements topic: img labels Sep 30, 2019
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, modulo some formatting nits I can push a fixup for. @domenic do you want to review this? Anyone else?

@domenic
Copy link
Member

domenic commented Sep 30, 2019

I'm on vacation so happy to leave this to you and/or others.

@annevk
Copy link
Member

annevk commented Sep 30, 2019

@fantasai @cbiesinger any thoughts on this? If not I plan to land this Wednesday.

@fantasai
Copy link
Contributor

fantasai commented Sep 30, 2019

Looks reasonable to me. I'd link to https://www.w3.org/TR/css-images-3/ instead, but, uh, I forgot to submit the publication request last month so it's not up yet. ^_^;;;;; Will be fixed hopefully sometime next week.. w3c/transitions#170

@domenic
Copy link
Member

domenic commented Sep 30, 2019

We dont, as a rule, link to /TR/ documents from WHATWG specs, so no rush there :).

Copy link
Contributor

@fantasai fantasai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to only apply to IMG, but should probably also apply to VIDEO?

@cbiesinger
Copy link

Looks good to me. We may want to include video but neither Blink nor Gecko support that right now.

@emilio
Copy link
Contributor Author

emilio commented Oct 2, 2019

Seems to only apply to IMG, but should probably also apply to VIDEO?

Yeah, we should probably add <video> too, and maybe other things, like <object> / <embed> / <input type="image"> / etc...?

Hoewever <img> is the most useful (as in broadly used), and the easiest to implement interoperably.

Also <video> has the extra trickiness of the poster image affecting its intrinsic size (which is just weird), and <img> has the "current" vs. "pending" request stuff, so it'd probably be quite different spec text anyway.

So yeah I'd rather land and ship <img>, then also implement and spec <video> (I'll file a follow-up on this repo to do that).

@annevk
Copy link
Member

annevk commented Oct 2, 2019

Filed #4961 as follow-up.

@annevk annevk merged commit 17a3f3c into whatwg:master Oct 2, 2019
@emilio emilio deleted the img-aspect-ratio branch October 2, 2019 12:56
@fantasai
Copy link
Contributor

fantasai commented Oct 3, 2019

@emilio input type=image certainly, but embed/object are also used for things which do not generally have an intrinsic aspect ratio, so I wouldn't apply it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: img
Development

Successfully merging this pull request may close these issues.

5 participants