706731 - Add support for defaultMuted attribute

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Andrew Quartey [:drexler]]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111104165243

Steps to reproduce:

This is a follow up from Bug 689834

Comment 1

[Andrew Quartey [:drexler]]

Attachment: Support for defaultMuted attribute v1

Comment 2

[Andrew Quartey [:drexler]]

Unsure whether i could say it fully depended on the 689834 as NS_IMPL_BOOL_ATTR needed it for resolving the muted attribute passed to it.

Comment 3

[Matthew Gregan [:kinetik]]

(In reply to andrew (:drexler) from comment #2)
> Unsure whether i could say it fully depended on the 689834 as
> NS_IMPL_BOOL_ATTR needed it for resolving the muted attribute passed to it.

Right, this patch depends on that patch because you need the muted atom.  I've readded it.  I thought it'd be simpler to split this out from bug 689834, but I was probably wrong as the trickier part is actually that bug and splitting things may have added to the confusion, sorry.

+ Support for defaultMuted attribute

Please include the bug number in the commit message.

+[scriptable, uuid(dcf812f5-373b-4Fc1-9a60-59078af33bf7)] 

Why's 'F' in caps?

Comment 4

[Andrew Quartey [:drexler]]

(In reply to Matthew Gregan [:kinetik] from comment #3)
> +[scriptable, uuid(dcf812f5-373b-4Fc1-9a60-59078af33bf7)] 
> 
> Why's 'F' in caps?

Oh missed that. Visual Studio's GUID generator has it in caps so manually changed it. I will resubmit the patch with the changes.

Comment 5

[Andrew Quartey [:drexler]]

Attachment: Support for defaultMuted attribute v2

Comment 6

[Matthew Gregan [:kinetik]]

Can you please write a mochitest and add it to content/media/test?  If you need a base, test_paused.html is a simple one to start from.  The main things to test is that <audio> and <audio muted> start in the expected states, and that toggling defaultMuted does not affect muted (and vice versa).

Comment 7

[Andrew Quartey [:drexler]]

Sure will do that.

Comment 8

[Andrew Quartey [:drexler]]

Attachment: Patch v3

defaultMuted attribute implementation and associated test.

Comment 9

[Matthew Gregan [:kinetik]]

Looks good, thanks.  I'll hang off on r+ing until we've got bug 689834 sorted out.

Comment 10

[Matthew Gregan [:kinetik]]

+    v1.defaultMuted = true;
+    a1.defaultMuted = true;
+    ok(v1.defaultMuted, "v1.defaultMuted should be true");
+    ok(a1.defaultMuted, "a1.defaultMuted should be true");

Can you also add a hasAttribute("muted") test here.  That'll prove that the defaultMuted DOM attribute is correctly mapped to the muted content attribute.  Also, check that the muted DOM attribute remains false.

+    v2.defaultMuted = false;
+    a2.defaultMuted = false;
+    ok(!v2.defaultMuted, "v2.defaultMuted should be false");
+    ok(v2.muted, "v2.muted should be true");
+    ok(!a2.defaultMuted, "a2.defaultMuted should be false");
+    ok(a2.muted, "a2.muted should be true");

And for this, a !hasAttribute("muted").

Comment 11

[Andrew Quartey [:drexler]]

Attachment: Patch v4

Added hasAttribute() checks to test_defaultMuted.html

Comment 12

[Andrew Quartey [:drexler]]

Attachment: Patch v5

Added extra checks for state of muted attribute in test_defaultMuted.html per bz's suggestion in comment 35 of bug 689834.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 581844 [details] [diff] [review]
Patch v5

r=me

Comment 14

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/20dcfdaaacec

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/20dcfdaaacec

Comment 16

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en/DOM/HTMLMediaElement
https://developer.mozilla.org/en/Firefox_11_for_developers

Also added the "muted" attribute on:
https://developer.mozilla.org/en/HTML/Element/video
https://developer.mozilla.org/en/HTML/Element/audio