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 various non-img elements #4961

Closed
annevk opened this issue Oct 2, 2019 · 13 comments · Fixed by #6529
Closed

Use width and height as intrinsic aspect ratio for various non-img elements #4961

annevk opened this issue Oct 2, 2019 · 13 comments · Fixed by #6529
Assignees
Labels
addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan topic: img topic: media topic: rendering

Comments

@annevk
Copy link
Member

annevk commented Oct 2, 2019

#4952 added support for a fallback aspect ratio for <img>, but as noted there by @cbiesinger @emilio @fantasai it might be good to also support <video>, <input type=image>, and perhaps other elements.

#3090 has a similar suggestion for <video> that should probably be resolved while fixing this.

@fantasai
Copy link
Contributor

fantasai commented Oct 3, 2019

VIDEO should definitely be included here, and also INPUT type=image. EMBED and OBJECT are sometimes used for things which do not have an intrinsic aspect ratio, so they probably should not get this behavior.

@AndresInSpace
Copy link

AndresInSpace commented Oct 3, 2019

As suggested, I made a new proposal for aspect-ratio in regards to art-direction with multiple sources, each having their own aspect-ratio.

@annevk annevk added the needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan label Oct 6, 2019
@jensimmons
Copy link

jensimmons commented Oct 7, 2019

This should apply to video and perhaps input type=image. The poster image for video should be considered.

It should not apply to any element that does not have an intrinsic aspect ratio — like iframe, embed or object. That would break web compat — causing a different layout result compared to how things work now. If you put a YouTube video on a webpage with an IFRAME right now, for instance. And give it width: 100%; height: auto it will not maintain its aspect ratio.

Needing a way to easily set an aspect ratio on iframe is what the aspect-ratio CSS property is for. This will give authors a way to say — "hey that YouTube video is 16x9, please adjust the height appropriately, as the width changes, maintaining a 16x9 aspect ratio".

TL;DR — this should apply to all elements that have an intrinsic aspect ratio, and none of the ones that do not.

@cbiesinger
Copy link

<embed>/<object> does sometimes have an aspect ratio, depending on what's loaded in it, no?

@jensimmons
Copy link

jensimmons commented Oct 7, 2019

If any element, including embed or object, is getting its sizing from a parent container, or children content, then that will be part of the layout calculation in CSS. But the elements themselves do not have an intrinsic aspect ratio. https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes

@emilio
Copy link
Contributor

emilio commented Oct 8, 2019

They do have an intrinsic aspect ratio in all engines, as far as I can tell. Trivial example:

data:text/html,<!doctype html><embed width=96 height=96 style="width: 100%; height: auto" type="image/png" src="https://w3c-test.org/images/blue96x96.png"></embed>

@annevk
Copy link
Member Author

annevk commented Oct 8, 2019

Yeah, embed and object are quite special and only sometimes act like iframe. I don't think we should change anything for them though as they are already special enough and we should not encourage further usage of them. (They probably ought to be marked deprecated/obsolete in the standard.)

bertogg pushed a commit to Igalia/webkit that referenced this issue Jan 16, 2020
…img>

https://bugs.webkit.org/show_bug.cgi?id=201641

Reviewed by Frédéric Wang

LayoutTests/imported/w3c:

assert_ratio(images[5], 133/106) in img-aspect-ratio.html still fails because of bug 206161.

* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/canvas-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/content-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html:

Source/WebCore:

According to [1], if HTML width and height attributes have valid values, not a percentage, and non-zero,
the value width/height is the default intrinsic aspect ratio for an <img> element. This will help to calculate
img element's layout size before loading. The value will be overridden if img is loaded. Also see [2].
This is currently limited in <img> element. Other elements like <canvas>, <video> and <input type=image>,
currently their aspect-ratio won't be affected.[3] While <picture> is still under discuss.[4]

[1]: https://html.spec.whatwg.org/multipage/rendering.html#attributes-for-embedded-content-and-images
[2]: WICG/intrinsicsize-attribute#16
[3]: whatwg/html#4961
[4]: whatwg/html#4968

Tests: imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/canvas-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/content-aspect-ratio.html

* page/Settings.yaml:
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const):

Source/WebKit:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* Shared/WebPreferences.yaml:

Source/WebKitLegacy/mac:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* WebView/WebPreferenceKeysPrivate.h:
* WebView/WebPreferences.mm:
([WebPreferences initialize]):
(-[WebPreferences aspectRatioOfImgFromWidthAndHeightEnabled]):
(-[WebPreferences setAspectRatioOfImgFromWidthAndHeightEnabled:]):
* WebView/WebPreferencesPrivate.h:
* WebView/WebView.mm:
(-[WebView _preferencesChanged:]):

Source/WebKitLegacy/win:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* Interfaces/IWebPreferencesPrivate.idl:
* WebPreferenceKeysPrivate.h:
* WebPreferences.cpp:
(WebPreferences::initializeDefaultSettings):
(WebPreferences::aspectRatioOfImgFromWidthAndHeightEnabled):
(WebPreferences::setAspectRatioOfImgFromWidthAndHeightEnabled):
* WebPreferences.h:
* WebView.cpp:
(WebView::notifyPreferencesChanged):

Tools:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

(enableExperimentalFeatures):
(setWebPreferencesForTestOptions):
* DumpRenderTree/win/DumpRenderTree.cpp:
(enableExperimentalFeatures):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254669 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…img>

https://bugs.webkit.org/show_bug.cgi?id=201641

Reviewed by Frédéric Wang

LayoutTests/imported/w3c:

assert_ratio(images[5], 133/106) in img-aspect-ratio.html still fails because of bug 206161.

* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/canvas-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/content-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.html:
* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html:

Source/WebCore:

According to [1], if HTML width and height attributes have valid values, not a percentage, and non-zero,
the value width/height is the default intrinsic aspect ratio for an <img> element. This will help to calculate
img element's layout size before loading. The value will be overridden if img is loaded. Also see [2].
This is currently limited in <img> element. Other elements like <canvas>, <video> and <input type=image>,
currently their aspect-ratio won't be affected.[3] While <picture> is still under discuss.[4]

[1]: https://html.spec.whatwg.org/multipage/rendering.html#attributes-for-embedded-content-and-images
[2]: WICG/intrinsicsize-attribute#16
[3]: whatwg/html#4961
[4]: whatwg/html#4968

Tests: imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/canvas-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html
       imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/content-aspect-ratio.html

* page/Settings.yaml:
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const):

Source/WebKit:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* Shared/WebPreferences.yaml:

Source/WebKitLegacy/mac:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* WebView/WebPreferenceKeysPrivate.h:
* WebView/WebPreferences.mm:
([WebPreferences initialize]):
(-[WebPreferences aspectRatioOfImgFromWidthAndHeightEnabled]):
(-[WebPreferences setAspectRatioOfImgFromWidthAndHeightEnabled:]):
* WebView/WebPreferencesPrivate.h:
* WebView/WebView.mm:
(-[WebView _preferencesChanged:]):

Source/WebKitLegacy/win:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

* Interfaces/IWebPreferencesPrivate.idl:
* WebPreferenceKeysPrivate.h:
* WebPreferences.cpp:
(WebPreferences::initializeDefaultSettings):
(WebPreferences::aspectRatioOfImgFromWidthAndHeightEnabled):
(WebPreferences::setAspectRatioOfImgFromWidthAndHeightEnabled):
* WebPreferences.h:
* WebView.cpp:
(WebView::notifyPreferencesChanged):

Tools:

Add experimental flag: AspectRatioOfImgFromWidthAndHeightEnabled.

(enableExperimentalFeatures):
(setWebPreferencesForTestOptions):
* DumpRenderTree/win/DumpRenderTree.cpp:
(enableExperimentalFeatures):


Canonical link: https://commits.webkit.org/219419@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254669 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@emilio
Copy link
Contributor

emilio commented Mar 24, 2021

This was resolved by #6032 I think, <video> and <canvas> should work per spec now. @annevk should we close this?

@emilio
Copy link
Contributor

emilio commented Mar 24, 2021

Actually they don't. @cbiesinger where is it defined that <canvas width height> maps to aspect-ratio? You landed a test in web-platform-tests/wpt#27349 claiming it matched the PR mentioned above, but I don't see how?

@annevk
Copy link
Member Author

annevk commented Mar 24, 2021

The rendering section has:

The width and height attributes map to the aspect-ratio property on img, canvas, and video elements, and input elements with a type attribute in the Image Button state.

As you note in #6527 that leaves something to be desired, but it's probably sufficient to close this issue?

@annevk
Copy link
Member Author

annevk commented Mar 24, 2021

Although I guess one weird thing is that for canvas they are not attr-dim-width but attr-canvas-width. That also seems like it needs some cleaning up.

@emilio
Copy link
Contributor

emilio commented Mar 24, 2021

Indeed. That's related to #6527, we don't really want to parse the attribute twice.

@cbiesinger
Copy link

Yes, what Anne said.

annevk added a commit that referenced this issue Mar 25, 2021
Also map <canvas width height> to aspect-ratio consistently with how the attributes are parsed.

Tests: web-platform-tests/wpt#28229.

Follow-up: #6528.

Closes #4961 and closes #6527.
annevk added a commit that referenced this issue May 12, 2021
Also map <canvas width height> to aspect-ratio consistently with how the attributes are parsed and clean up some wording.

Tests: web-platform-tests/wpt#28229 & web-platform-tests/wpt#28932.

Follow-up: #6528.

Closes #4961 and closes #6527.
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 needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan topic: img topic: media topic: rendering
Development

Successfully merging a pull request may close this issue.

6 participants