Bug 119537 – The muted attribute in media element is not set before loading resource.

• Status: UNCONFIRMED
• Component: Media

[Sanghyun Park]

Muted attribute in media element is not set before loading resource.
If we would like to check the muted attribute of media element before loading resource, we cannot check this attribute.

eq.,
<video id="m" muted></video>

At this time,  "document.getElementById("m").muted" returns the "false".

[Sanghyun Park]

Created attachment 208241 [details]
Patch

[Build Bot]

Comment on attachment 208241 [details]
Patch

Attachment 208241 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1416136

New failing tests:
media/video-muted-attribute.html
media/video-defaultmuted.html

[Build Bot]

Created attachment 208244 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[Build Bot]

Comment on attachment 208241 [details]
Patch

Attachment 208241 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1370726

New failing tests:
media/video-muted-attribute.html
media/video-defaultmuted.html

[Build Bot]

Created attachment 208246 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[Sanghyun Park]

Created attachment 208247 [details]
Patch

[Philippe Normand]

Quoting the spec: " Initially, the audio output should not be muted (false), but user agents may remember the last set value across sessions, on a per-site basis or otherwise, so the muted state may start as muted (true). "

So, not sure this patch really makes sense at HTMLMediaElement level. What do you think Eric?

[Sanghyun Park]

(In reply to comment #7)
> Quoting the spec: " Initially, the audio output should not be muted (false), but user agents may remember the last set value across sessions, on a per-site basis or otherwise, so the muted state may start as muted (true). "
> 
> So, not sure this patch really makes sense at HTMLMediaElement level. What do you think Eric?

Thanks for your concern.

But, the specification[1] is also described below.
"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. "
[1] http://www.w3.org/TR/html5/embedded-content-0.html#attr-media-muted

[Jer Noble]

Comment on attachment 208247 [details]
Patch

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

> LayoutTests/media/video-defaultmuted.html:27
>                  consoleWrite("<br>*** Test before setting src, IDL attribute should default to false");
> -                testMuted(false, defaultMuted);
> +                testMuted(defaultMuted, defaultMuted);

As you're changing the test, please also change the description above the test.  It no longer accurately describes the test that follows.

> LayoutTests/media/video-defaultmuted.html:41
> -                    testMuted(defaultMuted, newDefaultMuted);
> +                    // If muted attribute is existed, changed value of defaultMuted do not reflect the muted attribute.
> +                    // if muted atrribute is not existed, value of defaultMuted reflect the muted attribute.
> +                    testMuted(true, newDefaultMuted);

These comments do not make sense. I think I understand what you're trying to say here, but if it's true that "video.muted == true" in all cases, this belongs in the consoleWrite() above.

> LayoutTests/media/video-defaultmuted.html:55
> -                    testMuted(false, video.hasAttribute('muted'));
> +                    testMuted(!defaultMuted, video.hasAttribute('muted'));

Again, this no longer matches the message above.

[Eric Carlson]

Comment on attachment 208247 [details]
Patch

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.