- Revision
- 162941
- Author
- jer.no...@apple.com
- Date
- 2014-01-28 09:49:58 -0800 (Tue, 28 Jan 2014)
Log Message
Setting muted attribute on <video> element is not reflected in controls
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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (162940 => 162941)
--- trunk/LayoutTests/ChangeLog 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/LayoutTests/ChangeLog 2014-01-28 17:49:58 UTC (rev 162941)
@@ -1,3 +1,13 @@
+2014-01-28 Jer Noble <jer.no...@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....@apple.com>
Jettison DFG code when neither breakpoints or the profiler are active.
Modified: trunk/LayoutTests/media/video-defaultmuted-expected.txt (162940 => 162941)
--- trunk/LayoutTests/media/video-defaultmuted-expected.txt 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/LayoutTests/media/video-defaultmuted-expected.txt 2014-01-28 17:49:58 UTC (rev 162941)
@@ -8,16 +8,12 @@
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 @@
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 @@
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
Modified: trunk/LayoutTests/media/video-defaultmuted.html (162940 => 162941)
--- trunk/LayoutTests/media/video-defaultmuted.html 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/LayoutTests/media/video-defaultmuted.html 2014-01-28 17:49:58 UTC (rev 162941)
@@ -12,33 +12,73 @@
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);
@@ -60,7 +100,7 @@
function runNextTest()
{
- if (video) {
+ if (video && video.parentNode) {
video.parentNode.removeChild(video);
video = null;
}
@@ -74,6 +114,12 @@
test(false);
break;
case 3:
+ testAddedToDocument();
+ break;
+ case 4:
+ testExplicitlySetBeforeAddedToDocument();
+ break;
+ case 5:
consoleWrite("");
endTest();
break;
Modified: trunk/Source/WebCore/ChangeLog (162940 => 162941)
--- trunk/Source/WebCore/ChangeLog 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/Source/WebCore/ChangeLog 2014-01-28 17:49:58 UTC (rev 162941)
@@ -1,3 +1,35 @@
+2014-01-28 Jer Noble <jer.no...@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 <ander...@apple.com>
Add stubbed out VisitedLinkProvider class
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (162940 => 162941)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-01-28 17:49:58 UTC (rev 162941)
@@ -301,6 +301,7 @@
, 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 @@
scheduleDelayedAction(LoadMediaResource);
}
+ if (!m_explicitlyMuted) {
+ m_explicitlyMuted = true;
+ m_muted = fastHasAttribute(mutedAttr);
+ }
+
configureMediaControls();
return InsertionDone;
}
@@ -1215,8 +1221,11 @@
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 @@
bool HTMLMediaElement::muted() const
{
- return m_muted;
+ return m_explicitlyMuted ? m_muted : fastHasAttribute(mutedAttr);
}
void HTMLMediaElement::setMuted(bool muted)
@@ -2907,8 +2916,9 @@
#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 @@
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 @@
// 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();
}
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (162940 => 162941)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2014-01-28 17:43:07 UTC (rev 162940)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2014-01-28 17:49:58 UTC (rev 162941)
@@ -749,6 +749,7 @@
bool m_inActiveDocument : 1;
bool m_autoplaying : 1;
bool m_muted : 1;
+ bool m_explicitlyMuted : 1;
bool m_paused : 1;
bool m_seeking : 1;
