[webkit-reviews] review denied: [Bug 54634] media/video-controls-in-media-document.html has image+text diffs : [Attachment 89028] Test rewritten to use shadow DOM explicitly.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 11 11:12:35 PDT 2011


Eric Carlson <eric.carlson at apple.com> has denied Ami Fischman
<fischman at chromium.org>'s request for review:
Bug 54634: media/video-controls-in-media-document.html has image+text diffs
https://bugs.webkit.org/show_bug.cgi?id=54634

Attachment 89028: Test rewritten to use shadow DOM explicitly.
https://bugs.webkit.org/attachment.cgi?id=89028&action=review

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

> LayoutTests/media/video-controls-in-media-document.html:15
> +    var controls =
layoutTestController.shadowRoot(video).firstChild.firstChild;
> +    testExpected(controls.offsetTop, 208);  // == 240(video height) minus 32
(controls height).
> +    layoutTestController.notifyDone();

Hard coding the size of the controls doesn't seem like a great idea as ports
are free to use different sizes. Because this tests is to make sure controls
are rendered on top of the <video> element, why not just test that the top of
the controls element is above the bottom of the video element?

> LayoutTests/platform/mac/Skipped:2
> -# Copyright (C) 2008 Apple Inc. All rights reserved.
> +# Copyright (C) 2011 Apple Inc. All rights reserved.

An Apple notice copyright should include all years, eg. "2008, 2009, 2010,
2011".


More information about the webkit-reviews mailing list