[webkit-reviews] review denied: [Bug 119537] The muted attribute in media element is not set before loading resource. : [Attachment 208247] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 7 09:37:55 PDT 2013
Eric Carlson <eric.carlson at apple.com> has denied Sanghyun Park
<sh919.park at samsung.com>'s request for review:
Bug 119537: The muted attribute in media element is not set before loading
resource.
https://bugs.webkit.org/show_bug.cgi?id=119537
Attachment 208247: Patch
https://bugs.webkit.org/attachment.cgi?id=208247&action=review
------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=208247&action=review
Thank you for fixing this!
The patch is close, but not quite there yet.
> Source/WebCore/ChangeLog:14
> + (WebCore::HTMLMediaElement::parseAttribute):
> + (WebCore::HTMLMediaElement::loadResource):
Please add comments describing what changed in each method.
> LayoutTests/media/video-defaultmuted-expected.txt:11
> *** Test before setting src, IDL attribute should default to false
This comment is incorrect, maybe quote the spec text instead - "When a media
element is created, if it has a muted attribute specified, the user agent must
mute the media element's audio output, overriding any user preference."
> LayoutTests/media/video-defaultmuted-expected.txt:68
> *** Add 'muted' content attribute, it should have no effect on IDL attribute
> RUN(video.setAttribute('muted', 'muted'))
> -EXPECTED (video.muted == 'false') OK
> +EXPECTED (video.muted == 'true') OK
This no longer tests anything, because video.muted is already true before
setting the content attribute. Set video.muted to false before setting the
content attribute to make sure it is not changed.
More information about the webkit-reviews
mailing list