WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
34302
RenderVideo should prefer provided width/height properties to hard coded defaults when natural size is unavailable
https://bugs.webkit.org/show_bug.cgi?id=34302
Summary
RenderVideo should prefer provided width/height properties to hard coded defa...
Nick Young
Reported
2010-01-28 20:25:32 PST
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.
Attachments
Initial Patch.
(2.48 KB, patch)
2010-01-28 20:28 PST
,
Nick Young
eric.carlson
: review+
eric.carlson
: commit-queue-
Details
Formatted Diff
Diff
Updated Patch
(3.24 KB, patch)
2010-01-28 21:52 PST
,
Nick Young
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Nick Young
Comment 1
2010-01-28 20:28:38 PST
Created
attachment 47669
[details]
Initial Patch.
Eric Carlson
Comment 2
2010-01-28 21:42:47 PST
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
Comment 3
2010-01-28 21:52:03 PST
Created
attachment 47678
[details]
Updated Patch Comments reorganized and updated as requested.
Eric Carlson
Comment 4
2010-01-28 22:14:26 PST
Comment on
attachment 47678
[details]
Updated Patch Nice change, thanks! r=me
WebKit Commit Bot
Comment 5
2010-01-28 23:42:11 PST
Comment on
attachment 47678
[details]
Updated Patch Clearing flags on attachment: 47678 Committed
r54048
: <
http://trac.webkit.org/changeset/54048
>
WebKit Commit Bot
Comment 6
2010-01-28 23:42:17 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug