Setting muted attribute on <video> element is not reflected in controls
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2014 17:49:58 +0000 (17:49 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2014 17:49:58 +0000 (17:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=127726

Reviewed by Eric Carlson.

Source/WebCore:

If the 'muted' IDL attribute is queried after the 'muted' and 'src' content attributes are
set but before loading begins, it will return 'false' until loading begins, but with no
way to query whether that value is valid, nor firing a volumechange event when its value
changes.

The HTML spec says that the 'muted' content attribute controls whether audio output is
muted "when the media element is created", not when loading begins, but we don't
necessarily have a signal that the element is fully parsed.  Additionally, this means its
impossible to make an element via script and get this behavior.

So the new behavior is that the 'muted' IDL attribute will always reflect the 'muted'
content attribute up until one of the following three conditions:

- The 'muted' IDL attribute is set via script.
- The element is inserted in the document.
- The element begins loading.

After the first one of the above conditions, the 'muted' IDL attribute will no longer
change when the 'muted' content attribute changes.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::HTMLMediaElement):
(WebCore::HTMLMediaElement::parseAttribute):
* html/HTMLMediaElement.h:

LayoutTests:

* media/video-defaultmuted-expected.txt:
* media/video-defaultmuted.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@162941 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/video-defaultmuted-expected.txt
LayoutTests/media/video-defaultmuted.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h

index 3a60287..4f8625e 100644 (file)
@@ -1,3 +1,13 @@
+2014-01-28  Jer Noble  <jer.noble@apple.com>
+
+        Setting muted attribute on <video> element is not reflected in controls
+        https://bugs.webkit.org/show_bug.cgi?id=127726
+
+        Reviewed by Eric Carlson.
+
+        * media/video-defaultmuted-expected.txt:
+        * media/video-defaultmuted.html:
+
 2014-01-28  Mark Lam  <mark.lam@apple.com>
 
         Jettison DFG code when neither breakpoints or the profiler are active.
index d718364..3477164 100644 (file)
@@ -8,16 +8,12 @@ RUN(video = document.createElement('video'))
 RUN(video.setAttribute('controls', 'controls'))
 RUN(video.setAttribute('muted', 'muted'))
 
-*** Test before setting src, IDL attribute should default to false
-EXPECTED (video.muted == 'false') OK
+*** Test before setting src, muted IDL attribute should default to muted content attribute
+EXPECTED (video.muted == 'true') OK
 EXPECTED (video.defaultMuted == 'true') OK
 
 EVENT(loadedmetadata)
 
-*** After setting url, content attribute should have set IDL attribute
-EXPECTED (video.muted == 'true') OK
-EXPECTED (video.defaultMuted == 'true') OK
-
 *** Change 'defaultMuted', IDL attribute should not change but content attribute should.
 RUN(video.defaultMuted = false)
 EXPECTED (video.muted == 'true') OK
@@ -41,16 +37,12 @@ EXPECTED (video.defaultMuted == 'false') OK
 RUN(video = document.createElement('video'))
 RUN(video.setAttribute('controls', 'controls'))
 
-*** Test before setting src, IDL attribute should default to false
+*** Test before setting src, muted IDL attribute should default to muted content attribute
 EXPECTED (video.muted == 'false') OK
 EXPECTED (video.defaultMuted == 'false') OK
 
 EVENT(loadedmetadata)
 
-*** After setting url, content attribute should have set IDL attribute
-EXPECTED (video.muted == 'false') OK
-EXPECTED (video.defaultMuted == 'false') OK
-
 *** Change 'defaultMuted', IDL attribute should not change but content attribute should.
 RUN(video.defaultMuted = true)
 EXPECTED (video.muted == 'false') OK
@@ -68,5 +60,41 @@ RUN(video.setAttribute('muted', 'muted'))
 EXPECTED (video.muted == 'false') OK
 EXPECTED (video.defaultMuted == 'true') OK
 
+
+*** Test that the 'muted' content attribute reflects the 'muted' IDL attribute before the element is added to the document, and does not reflect after
+
+RUN(video = document.createElement('video'))
+RUN(video.setAttribute('controls', 'controls'))
+EXPECTED (video.muted == 'false') OK
+EXPECTED (video.defaultMuted == 'false') OK
+
+*** Change 'muted' content attribute, IDL attribute should change.
+RUN(video.setAttribute('muted', 'muted'))
+EXPECTED (video.muted == 'true') OK
+EXPECTED (video.defaultMuted == 'true') OK
+RUN(document.getElementById('parent').appendChild(video))
+
+*** Change 'muted' content attribute, IDL attribute should not change.
+EXPECTED (video.muted == 'true') OK
+EXPECTED (video.defaultMuted == 'false') OK
+
+
+*** Test that setting the 'muted' IDL attribute means that changes to the 'muted' content attribute are no longer reflected.
+
+RUN(video = document.createElement('video'))
+RUN(video.setAttribute('controls', 'controls'))
+EXPECTED (video.muted == 'false') OK
+EXPECTED (video.defaultMuted == 'false') OK
+
+*** Change 'muted' content attribute, IDL attribute should change.
+RUN(video.setAttribute('muted', 'muted'))
+EXPECTED (video.muted == 'true') OK
+EXPECTED (video.defaultMuted == 'true') OK
+
+*** Change 'muted' IDL attribute, then the content attribute. IDL attribute should not change.
+RUN(video.muted = true)
+EXPECTED (video.muted == 'true') OK
+EXPECTED (video.defaultMuted == 'false') OK
+
 END OF TEST
 
index 931c0a2..5da6271 100644 (file)
                 testExpected("video.defaultMuted", expectedDefaultMuted);
             }
 
+            function testAddedToDocument()
+            {
+                consoleWrite("<br><br><em>*** Test that the 'muted' content attribute reflects the 'muted' IDL attribute before the element"
+                    + " is added to the document, and does not reflect after</em></br>");
+
+                run("video = document.createElement('video')");
+                run("video.setAttribute('controls', 'controls')");
+                video.setAttribute('width', '300');
+                testMuted(false, false);
+
+                consoleWrite("<br>*** Change 'muted' content attribute, IDL attribute should change.");
+                run("video.setAttribute('muted', 'muted')");
+                testMuted(true, true);
+                run("document.getElementById('parent').appendChild(video)");
+
+                consoleWrite("<br>*** Change 'muted' content attribute, IDL attribute should not change.");
+                video.removeAttribute('muted');
+                testMuted(true, false);
+
+                runNextTest();
+            }
+
+            function testExplicitlySetBeforeAddedToDocument()
+            {
+                consoleWrite("<br><br><em>*** Test that setting the 'muted' IDL attribute means that changes to "
+                    + "the 'muted' content attribute are no longer reflected.</em></br>");
+
+                run("video = document.createElement('video')");
+                run("video.setAttribute('controls', 'controls')");
+                video.setAttribute('width', '300');
+                testMuted(false, false);
+
+                consoleWrite("<br>*** Change 'muted' content attribute, IDL attribute should change.");
+                run("video.setAttribute('muted', 'muted')");
+                testMuted(true, true);
+
+                consoleWrite("<br>*** Change 'muted' IDL attribute, then the content attribute. IDL attribute should not change.");
+                run("video.muted = true");
+                video.removeAttribute('muted');
+                testMuted(true, false);
+
+                runNextTest();
+            }
+
             function test(defaultMuted)
             {
-                consoleWrite("<br><br><b>*** Test <em>" + (defaultMuted ? "with" : "without") + "</em> 'muted' content attribute</b><br>");
+                consoleWrite("<br><br><em>*** Test <em>" + (defaultMuted ? "with" : "without") + "</em> 'muted' content attribute</em><br>");
 
                 run("video = document.createElement('video')");
                 run("video.setAttribute('controls', 'controls')");
                 video.setAttribute('width', '300');
                 if (defaultMuted)
                     run("video.setAttribute('muted', 'muted')");
-                document.getElementById('parent').appendChild(video);
 
-                consoleWrite("<br>*** Test before setting src, IDL attribute should default to false");
-                testMuted(false, defaultMuted);
+                consoleWrite("<br>*** Test before setting src, muted IDL attribute should default to muted content attribute");
+                testMuted(defaultMuted, defaultMuted);
 
                 var loadedmetadata = function(evt)
                 {
                     consoleWrite("<br>EVENT(" + evt.type + ")");
 
-                    consoleWrite("<br>*** After setting url, content attribute should have set IDL attribute");
-                    testMuted(defaultMuted, defaultMuted);
-
                     consoleWrite("<br>*** Change 'defaultMuted', IDL attribute should not change but content attribute should.");
                     var newDefaultMuted = !defaultMuted;
                     run("video.defaultMuted = " + newDefaultMuted);
                     testMuted(defaultMuted, newDefaultMuted);
                     testExpected("video.hasAttribute('muted')", newDefaultMuted);
-    
+
                     consoleWrite("<br>*** Change 'muted' IDL attribute, content attribute should not change");
                     run("video.muted = false");
                     testMuted(false, newDefaultMuted);
 
             function runNextTest()
             {
-                if (video) {
+                if (video && video.parentNode) {
                     video.parentNode.removeChild(video);
                     video = null;
                 }
                     test(false);
                     break;
                 case 3:
+                    testAddedToDocument();
+                    break;
+                case 4:
+                    testExplicitlySetBeforeAddedToDocument();
+                    break;
+                case 5:
                     consoleWrite("");
                     endTest();
                     break;
index 6e51278..0d3a28b 100644 (file)
@@ -1,3 +1,35 @@
+2014-01-28  Jer Noble  <jer.noble@apple.com>
+
+        Setting muted attribute on <video> element is not reflected in controls
+        https://bugs.webkit.org/show_bug.cgi?id=127726
+
+        Reviewed by Eric Carlson.
+
+        If the 'muted' IDL attribute is queried after the 'muted' and 'src' content attributes are
+        set but before loading begins, it will return 'false' until loading begins, but with no
+        way to query whether that value is valid, nor firing a volumechange event when its value
+        changes.
+
+        The HTML spec says that the 'muted' content attribute controls whether audio output is
+        muted "when the media element is created", not when loading begins, but we don't
+        necessarily have a signal that the element is fully parsed.  Additionally, this means its
+        impossible to make an element via script and get this behavior.
+
+        So the new behavior is that the 'muted' IDL attribute will always reflect the 'muted'
+        content attribute up until one of the following three conditions:
+
+        - The 'muted' IDL attribute is set via script.
+        - The element is inserted in the document.
+        - The element begins loading.
+
+        After the first one of the above conditions, the 'muted' IDL attribute will no longer
+        change when the 'muted' content attribute changes.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::HTMLMediaElement):
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * html/HTMLMediaElement.h:
+
 2014-01-28  Anders Carlsson  <andersca@apple.com>
 
         Add stubbed out VisitedLinkProvider class
index 8b0fcb2..71baf40 100644 (file)
@@ -301,6 +301,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_inActiveDocument(true)
     , m_autoplaying(true)
     , m_muted(false)
+    , m_explicitlyMuted(false)
     , m_paused(true)
     , m_seeking(false)
     , m_sentStalledEvent(false)
@@ -695,6 +696,11 @@ Node::InsertionNotificationRequest HTMLMediaElement::insertedInto(ContainerNode&
             scheduleDelayedAction(LoadMediaResource);
     }
 
+    if (!m_explicitlyMuted) {
+        m_explicitlyMuted = true;
+        m_muted = fastHasAttribute(mutedAttr);
+    }
+
     configureMediaControls();
     return InsertionDone;
 }
@@ -1215,8 +1221,11 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT
         m_player->setPreload(m_mediaSession->effectivePreloadForElement(*this));
     m_player->setPreservesPitch(m_webkitPreservesPitch);
 
-    if (fastHasAttribute(mutedAttr))
-        m_muted = true;
+    if (!m_explicitlyMuted) {
+        m_explicitlyMuted = true;
+        m_muted = fastHasAttribute(mutedAttr);
+    }
+
     updateVolume();
 
 #if ENABLE(MEDIA_SOURCE)
@@ -2897,7 +2906,7 @@ void HTMLMediaElement::setVolume(double vol, ExceptionCode& ec)
 
 bool HTMLMediaElement::muted() const
 {
-    return m_muted;
+    return m_explicitlyMuted ? m_muted : fastHasAttribute(mutedAttr);
 }
 
 void HTMLMediaElement::setMuted(bool muted)
@@ -2907,8 +2916,9 @@ void HTMLMediaElement::setMuted(bool muted)
 #if PLATFORM(IOS)
     UNUSED_PARAM(muted);
 #else
-    if (m_muted != muted) {
+    if (m_muted != muted || !m_explicitlyMuted) {
         m_muted = muted;
+        m_explicitlyMuted = true;
         // Avoid recursion when the player reports volume changes.
         if (!processingMediaPlayerCallback()) {
             if (m_player) {
@@ -4211,7 +4221,7 @@ void HTMLMediaElement::updateVolume()
     if (!processingMediaPlayerCallback()) {
         Page* page = document().page();
         double volumeMultiplier = page ? page->mediaVolume() : 1;
-        bool shouldMute = m_muted;
+        bool shouldMute = muted();
 
         if (m_mediaController) {
             volumeMultiplier *= m_mediaController->volume();
@@ -4272,7 +4282,7 @@ void HTMLMediaElement::updatePlayState()
             // Set rate, muted before calling play in case they were set before the media engine was setup.
             // The media engine should just stash the rate and muted values since it isn't already playing.
             m_player->setRate(m_playbackRate);
-            m_player->setMuted(m_muted);
+            m_player->setMuted(muted());
 
             m_player->play();
         }
index f724701..07faa23 100644 (file)
@@ -749,6 +749,7 @@ private:
     bool m_inActiveDocument : 1;
     bool m_autoplaying : 1;
     bool m_muted : 1;
+    bool m_explicitlyMuted : 1;
     bool m_paused : 1;
     bool m_seeking : 1;