[webkit-reviews] review denied: [Bug 57673] Media Element's muted value ignored if set before play : [Attachment 88547] Fix + manual test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 7 07:11:07 PDT 2011


Eric Carlson <eric.carlson at apple.com> has denied David Humphrey
<david.humphrey at senecac.on.ca>'s request for review:
Bug 57673: Media Element's muted value ignored if set before play
https://bugs.webkit.org/show_bug.cgi?id=57673

Attachment 88547: Fix + manual test
https://bugs.webkit.org/attachment.cgi?id=88547&action=review

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

"bugzilla-57673.html" won't be a terribly helpful name to someone browsing the
test directory later, maybe something like "media-muted.html" to follow the
pattern of several of the existing manual media tests?

This patch is missing the ChangeLog. If you aren't sure how to generate it,
http://www.webkit.org/coding/contributing.html has a good description. 

Also if you set the cq? flag when you set r?, the reviewer will set cq+ when
your patch is r+'ed and the commit bot will commit it automatically.

Marking r- because of the missing ChangeLog, but please also consider the
load() comment and test name suggestion.

> third_party/WebKit/Source/WebCore/manual-tests/bugzilla-57673.html:22
> +	       vid.src =
"http://src.chromium.org/svn/trunk/src/chrome/test/data/media/" +
findMediaFile("video", "bear");
> +
> +	       vid.load();

vid.load() is unnecessary, setting vid.src triggers the resource selection
algorithm.


More information about the webkit-reviews mailing list