Summary: | RenderVideo should prefer provided width/height properties to hard coded defaults when natural size is unavailable | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nick Young <nicholas.young> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Enhancement | CC: | commit-queue, eric.carlson | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Nick Young
2010-01-28 20:25:32 PST
Created attachment 47669 [details]
Initial Patch.
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 Created attachment 47678 [details]
Updated Patch
Comments reorganized and updated as requested.
Comment on attachment 47678 [details]
Updated Patch
Nice change, thanks!
r=me
Comment on attachment 47678 [details] Updated Patch Clearing flags on attachment: 47678 Committed r54048: <http://trac.webkit.org/changeset/54048> All reviewed patches have been landed. Closing bug. |