[webkit-reviews] review granted: [Bug 43702] <video> element does not resize correctly : [Attachment 63913] revised patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 9 12:02:35 PDT 2010
Darin Adler <darin at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 43702: <video> element does not resize correctly
https://bugs.webkit.org/show_bug.cgi?id=43702
Attachment 63913: revised patch
https://bugs.webkit.org/attachment.cgi?id=63913&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> }
> + else {
Else should go on the same line as the closing brace to match the usual WebKit
style.
> +void RenderVideo::updateIntrinsicSize()
> {
> + IntSize size = calculateIntrinsicSize();
> +
> + // Never set the element size to zero when in a media document.
> + if (size.isEmpty() && node()->ownerDocument() &&
node()->ownerDocument()->isMediaDocument())
> return;
> +
> if (size != intrinsicSize()) {
> setIntrinsicSize(size);
> setPrefWidthsDirty(true);
> setNeedsLayout(true);
> }
> }
Looks good. I would have used an early return style instead of nesting the work
inside an if statement.
> + if (errorOccurred())
> + setIntrinsicSize(defaultSize());
Could you just call updateIntrinsicSize here instead of explicitly setting the
size to default?
r=me
More information about the webkit-reviews
mailing list