[webkit-reviews] review denied: [Bug 64880] Controls not reset when media element url changed : [Attachment 104506] Fix for the bug and added a sample test page.

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Arko Saha
<nghq36 at motorola.com>'s request for review:
Bug 64880: Controls not reset when media element url changed
https://bugs.webkit.org/show_bug.cgi?id=64880

Attachment 104506: Fix for the bug and added a sample test page.
https://bugs.webkit.org/attachment.cgi?id=104506&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
Thanks for picking this up!


View in context: https://bugs.webkit.org/attachment.cgi?id=104506&action=review


> Source/WebCore/ChangeLog:7
> +
> +

Nit: The extra blank line is not necessary.


> Source/WebCore/manual-tests/media-controls-invalid-url.html:6
> +	 function setMovie(index)

Evil, evil tabs!


> Source/WebCore/manual-tests/media-controls-invalid-url.html:28
> +  <p>Test for <a
href="https://bugs.webkit.org/show_bug.cgi?id=64880">https://bugs.webkit.org/sh
ow_bug.cgi?id=64880</a><br>
> +    Media element controls are not reset to their default state when the src
is changed.</p>
> +  <video id="vid" height=480 type="video/mp4"
> +	    src="../../../LayoutTests/media/content/test.mp4"
> +	    controls>
> +  </video>
> +  <br>
> +  <div onclick="setMovie('1')" >Valid url</div>
> +  <div onclick="setMovie('0')" >Invalid url</div>

This should be made into an automated layout test. Manual tests aren't run very
often so the things they test for can regress - e.g..
https://bugs.webkit.org/show_bug.cgi?id=66303. 

Making this into an automated test should be easy, I would start out with a
valid url and change to an invalid one in a 'canplaythrough' event listener. 

It should be possible to be make the results platform agnostic, despite the
fact that ports have different control layout, by checking for the presence of
the slider with the shadow DOM API (look for other tests in media/ tests that
use media-controls.js).