According to the HTML5 spec, a <video> element's intrinsic size is: + the intrinsic width/height of a video element's playback area is the intrinsic width/height of the video resource, if that is available + otherwise it is the intrinsic width/height of the poster frame, if that is available; + otherwise it is 300/155 CSS pixels In WebKit a newly opened <video> element follows this rule, but if the 'poster' or 'src' attribute is changed, and the url fails to load the element will not be sized correctly.
Created attachment 63857 [details] proposed patch
Attachment 63857 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/html/HTMLVideoElement.cpp:111: An else should appear on the same line as the preceding } [whitespace/newline] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 63857 [details] proposed patch > +#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO) > + // Reset the intrinsic size in case we are loading a new movie. > + if (renderer() && renderer()->isVideo()) > + toRenderVideo(renderer())->videoSizeChanged(); > +#endif > + > if (renderer()) > renderer()->updateFromElement(); Why do we need a new function that's video-element specific? Why can't RenderVideo do this in an override of updateFromElement?
(In reply to comment #3) > (From update of attachment 63857 [details]) > > +#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO) > > + // Reset the intrinsic size in case we are loading a new movie. > > + if (renderer() && renderer()->isVideo()) > > + toRenderVideo(renderer())->videoSizeChanged(); > > +#endif > > + > > if (renderer()) > > renderer()->updateFromElement(); > > Why do we need a new function that's video-element specific? Why can't RenderVideo do this > in an override of updateFromElement? Excellent point, thanks!
Created attachment 63913 [details] revised patch RenderVideo::videoSizeChanged removed completely, updateFromElement now resets the intrinsic size when necessary.
Attachment 63913 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/html/HTMLVideoElement.cpp:111: An else should appear on the same line as the preceding } [whitespace/newline] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 63913 [details] revised patch > } > + 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
http://trac.webkit.org/changeset/64997, with all of Darin's suggestions.
Revision r64997 cherry-picked into qtwebkit-2.2 with commit 68a63b1 <http://gitorious.org/webkit/qtwebkit/commit/68a63b1>