Bug 34302 – RenderVideo should prefer provided width/height properties to hard coded defaults when natural size is unavailable

• Component: Media
• Status: RESOLVED FIXED

[Nick Young]

The constructor of RenderVideo in WebCore/rendering/RenderVideo.cpp attempts to determine the intrinsic size of the video by calling the naturalSize() method of the media player.
However, when RenderVideo is constructed the media player may not yet have been constructed.

In these situations, the renderer currently falls back to a hard coded default video size as the intrinsic size - (300, 150).
This patch contends that in these situations, the width and height properties of the video element, if available, should be used as the intrinsic size until better values are available.

If the width and height properties are unavailable, then the hard coded default is used once again.

[Nick Young]

Created attachment 47669 [details]
Initial Patch.

[Eric Carlson]

Comment on attachment 47669 [details]
Initial Patch.

>          // size since they also have audio thrown at them. By setting the intrinsic
>          // size to 300x1 the video will resize itself in these cases, and audio will
>          // have the correct height (it needs to be > 0 for controls to render properly).
> -        if (video->ownerDocument() && video->ownerDocument()->isMediaDocument())
> +        if (video->hasAttribute(widthAttr) && video->hasAttribute(heightAttr))
> +            setIntrinsicSize(IntSize(video->width(), video->height()));
> +        else if (video->ownerDocument() && video->ownerDocument()->isMediaDocument())

This comment doesn't make sense now, it is about the second test. Please add a comment about the logic in the first test ("Prefer provided video element width/height properties...") and move this comment down to be with the second test.

r=me with this change

[Nick Young]

Created attachment 47678 [details]
Updated Patch

Comments reorganized and updated as requested.

[Eric Carlson]

Comment on attachment 47678 [details]
Updated Patch

Nice change, thanks!

r=me

[WebKit Commit Bot]

Comment on attachment 47678 [details]
Updated Patch

Clearing flags on attachment: 47678

Committed r54048: <http://trac.webkit.org/changeset/54048>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.