Closed
Bug 706731
Opened 13 years ago
Closed 13 years ago
Add support for defaultMuted attribute
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: drexler, Assigned: drexler)
References
()
Details
(Keywords: dev-doc-complete, Whiteboard: [mentor=jdm] [lang=c++])
Attachments
(1 file, 4 obsolete files)
5.72 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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
Updated•13 years ago
|
Component: General → Video/Audio
Product: Firefox → Core
QA Contact: general → video.audio
Updated•13 years ago
|
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•13 years ago
|
Whiteboard: [mentor=jdm] [lang=c++]
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #578362 -
Flags: feedback?(josh)
Assignee | ||
Comment 2•13 years ago
|
||
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.
Updated•13 years ago
|
Attachment #578362 -
Flags: review?(kinetik)
Attachment #578362 -
Flags: feedback?(josh)
Attachment #578362 -
Flags: feedback+
Comment 3•13 years ago
|
||
(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?
Depends on: 689834
Assignee | ||
Comment 4•13 years ago
|
||
(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.
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #578362 -
Attachment is obsolete: true
Attachment #578362 -
Flags: review?(kinetik)
Attachment #578980 -
Flags: review?(kinetik)
Comment 6•13 years ago
|
||
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).
Assignee | ||
Comment 7•13 years ago
|
||
Sure will do that.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → andrew.quartey
Assignee | ||
Comment 8•13 years ago
|
||
defaultMuted attribute implementation and associated test.
Attachment #578980 -
Attachment is obsolete: true
Attachment #578980 -
Flags: review?(kinetik)
Attachment #579823 -
Flags: review?(kinetik)
Comment 9•13 years ago
|
||
Looks good, thanks. I'll hang off on r+ing until we've got bug 689834 sorted out.
Comment 10•13 years ago
|
||
+ 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").
Assignee | ||
Comment 11•13 years ago
|
||
Added hasAttribute() checks to test_defaultMuted.html
Attachment #579823 -
Attachment is obsolete: true
Attachment #579823 -
Flags: review?(kinetik)
Attachment #581535 -
Flags: review?(kinetik)
Updated•13 years ago
|
Attachment #581535 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 12•13 years ago
|
||
Added extra checks for state of muted attribute in test_defaultMuted.html per bz's suggestion in comment 35 of bug 689834.
Attachment #581535 -
Attachment is obsolete: true
Attachment #581844 -
Flags: review?(bzbarsky)
Comment 13•13 years ago
|
||
Comment on attachment 581844 [details] [diff] [review] Patch v5 r=me
Attachment #581844 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 15•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/20dcfdaaacec
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Comment 16•12 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•