[webkit-reviews] review granted: [Bug 89344] [Chromium] Handle smaller sizes of media elements in Chromium video controls : [Attachment 161440] rewrote tests

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 30 10:09:07 PDT 2012


Eric Carlson <eric.carlson at apple.com> has granted Silvia Pfeiffer
<silviapfeiffer1 at gmail.com>'s request for review:
Bug 89344: [Chromium] Handle smaller sizes of media elements in Chromium video
controls
https://bugs.webkit.org/show_bug.cgi?id=89344

Attachment 161440: rewrote tests
https://bugs.webkit.org/attachment.cgi?id=161440&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=161440&action=review


> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:123
> +	   // Shrink padding between 400px to 160px video width from 5px to
0px.

Nit: this comment does not describe this code.

> LayoutTests/platform/chromium/media/controls-video-sizes.html:31
> +	       for (var i = 0; i < videos.length;  i++) {
> +		   videos[i].src = "../../../media/content/test.ogv";
> +		   videos[i].addEventListener('canplaythrough', function () {
> +		       ready += 1;
> +		   }, false);
> +	       }
> +	       waitForAllVideosReady();

Nit: a more efficient way to do this would be to call "endTest()" from the
event listener once ready == videos.length instead of using a timer to watch
for the transition.


More information about the webkit-reviews mailing list