Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 310122 <video> width and height properties should not influence intrinsic width/height
Starred by 0 users Reported by phil...@opera.com, Oct 22 2013 Back to list
Status: Fixed
Owner:
Email to this user bounced
Closed: Oct 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment
Spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-map-element.html#dom-dim-width

RenderVideo::calculateIntrinsicSize() falls back to video->width() and height(), which isn't what the spec requires and also doesn't match what Gecko, Presto or IE11 does.

The code block should simply be removed.
 
Comment 1 by phil...@opera.com, Oct 22 2013
In addition, the spec has also changed with regards to the final fallback for intrinsic width and height: it used to be 300/150 CSS pixels, but the final step is now "otherwise the intrinsic width is missing". The difference is that the video won't have an intrinsic ratio, so a video with no src and only a width="100" attribute should now render as 100x150, where it will be 100x50 if the intrinsic size were 300x150.

Spec: Search for "intrinsic width of a video element" in http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html
Comment 2 by phil...@opera.com, Oct 22 2013
Finally, RenderVideo::calculateIntrinsicSize() checks the video size before the poster size, which is the opposite order from what the spec says. There will be a difference for a video where the poster size is different, but which has loaded metadata.
Comment 3 by phil...@opera.com, Oct 22 2013
Cc: tabatkins@chromium.org
Tab, this is the bug I bothered you about on IRC.
Comment 4 by phil...@opera.com, Oct 22 2013
Related spec issue: https://github.com/tabatkins/specs/issues/17
Project Member Comment 5 by bugdroid1@chromium.org, Oct 24 2013
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=160491

------------------------------------------------------------------------
r160491 | philipj@opera.com | 2013-10-24T20:28:22.529367Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderVideo.cpp?r1=160491&r2=160490&pathrev=160491
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/media/video-intrinsic-width-height-expected.txt?r1=160491&r2=160490&pathrev=160491
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/media/video-intrinsic-width-height.html?r1=160491&r2=160490&pathrev=160491

Remove <video> width and height attributes from intrisic size logic

The width/height content attributes already influence specified style
via HTMLVideoElement::collectStyleForPresentationAttribute, to also
influence the intrinsic size has never been part of the spec:
http://whatwg.org/html#the-video-element

The test case passes in Firefox Nightly and IE11 Release Preview,
but fails in Opera Presto, which has no default intrinsic size.
Opera is actually correct per spec, but fixing Blink can be done
in a separate bug.

BUG= 310122 

Review URL: https://codereview.chromium.org/38313002
------------------------------------------------------------------------
Comment 6 by phil...@opera.com, Oct 24 2013
Status: Fixed
Closing as fixed and leaving the rest to issue 311346.
Comment 7 by sshru...@google.com, Mar 21 2016
Components: -Blink>Video Blink>Media>Video
Renaming Blink>Video to Blink>Media>Video for better characterization
Sign in to add a comment