- Revision
- 213848
- Author
- [email protected]
- Date
- 2017-03-13 09:39:08 -0700 (Mon, 13 Mar 2017)
Log Message
[Modern Media Controls] Volume icon doesn't turn to mute when the knob is set to 0
https://bugs.webkit.org/show_bug.cgi?id=169553
<rdar://problem/30754543>
Patch by Antoine Quint <[email protected]> on 2017-03-13
Reviewed by Eric Carlson.
Source/WebCore:
When we start changing the volume using the mouse, we record the volume at that point
and as we drag the volume slider, we set the mute button to show that the media is
muted if the volume is 0, and set the actual media volume to be the initial volume
prior to dragging. This way, when we toggle the "muted" property by pressing the
mute button, the original volume is restored.
To function correctly, this required some changed to Slider. The volume slider would
fail to update if the value set was the same as the initial value since we would use
only the "change" event to identify the end of a slider drag interaction. This was
incorrect since if the initial value and the final value while dragging the slider
were the same, no "change" event would be fired. So we now use mouse events to
identify when the slider is being dragged.
Test: media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
* Modules/modern-media-controls/controls/slider.js:
(Slider.prototype.set value):
(Slider.prototype.handleEvent):
(Slider.prototype._handleMousedownEvent):
(Slider.prototype._handleInputEvent):
(Slider.prototype._handleMouseupEvent):
(Slider.prototype._handleChangeEvent): Deleted.
* Modules/modern-media-controls/controls/volume-slider.js:
(VolumeSlider):
(VolumeSlider.prototype.draw):
(VolumeSlider.prototype.handleEvent): Deleted.
* Modules/modern-media-controls/media/volume-support.js:
(VolumeSupport.prototype.controlValueWillStartChanging):
(VolumeSupport.prototype.controlValueDidChange):
LayoutTests:
Adding a new test where we drag the volume slider to 0 and ensure that the volume gets muted
and that clicking on the mute button resets the volume to be the same value as prior to the
dragging interaction.
* media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt: Added.
* media/modern-media-controls/volume-support/volume-support-drag-to-mute.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (213847 => 213848)
--- trunk/LayoutTests/ChangeLog 2017-03-13 16:29:22 UTC (rev 213847)
+++ trunk/LayoutTests/ChangeLog 2017-03-13 16:39:08 UTC (rev 213848)
@@ -1,3 +1,18 @@
+2017-03-13 Antoine Quint <[email protected]>
+
+ [Modern Media Controls] Volume icon doesn't turn to mute when the knob is set to 0
+ https://bugs.webkit.org/show_bug.cgi?id=169553
+ <rdar://problem/30754543>
+
+ Reviewed by Eric Carlson.
+
+ Adding a new test where we drag the volume slider to 0 and ensure that the volume gets muted
+ and that clicking on the mute button resets the volume to be the same value as prior to the
+ dragging interaction.
+
+ * media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt: Added.
+ * media/modern-media-controls/volume-support/volume-support-drag-to-mute.html: Added.
+
2017-03-13 Manuel Rego Casasnovas <[email protected]>
Unprefix -webkit-min-content, -webkit-max-content and -webkit-fit-content
Added: trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt (0 => 213848)
--- trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt 2017-03-13 16:39:08 UTC (rev 213848)
@@ -0,0 +1,22 @@
+Testing that dragging the volume slider to 0 turns the mute button on and that pressing the mute button resets the volume to the previous value.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS media.volume is 1
+PASS mediaController.controls.volumeSlider.value is 1
+PASS media.volume became 0.5
+PASS muteButton.getBoundingClientRect().width became different from 0
+PASS volumeSlider.getBoundingClientRect().width became different from 0
+
+We drag the volume slider to set the volume from 0.5 to 0
+PASS media.muted became true
+
+We click on the mute button which should set the volume back to 0.5
+PASS media.volume became 0.5
+PASS media.muted is false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute.html (0 => 213848)
--- trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-drag-to-mute.html 2017-03-13 16:39:08 UTC (rev 213848)
@@ -0,0 +1,87 @@
+<script src=""
+<script src="" type="text/_javascript_"></script>
+<script src="" type="text/_javascript_"></script>
+<body>
+<style type="text/css" media="screen">
+
+ video, #host {
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 800px;
+ height: 240px;
+ }
+
+</style>
+<video src="" autoplay controls></video>
+<div id="host"></div>
+<script type="text/_javascript_">
+
+window.jsTestIsAsync = true;
+
+description("Testing that dragging the volume slider to 0 turns the mute button on and that pressing the mute button resets the volume to the previous value.");
+
+const container = document.querySelector("div#host");
+const media = document.querySelector("video");
+const mediaController = createControls(container, media, null);
+
+const muteButton = mediaController.controls.muteButton.element;
+const volumeSlider = mediaController.controls.volumeSlider.children[1].element;
+
+function runTest()
+{
+ media.removeEventListener("play", runTest);
+ media.pause();
+
+ shouldBe("media.volume", "1");
+ shouldBe("mediaController.controls.volumeSlider.value", "1");
+
+ media.volume = 0.5;
+ shouldBecomeEqual("media.volume", "0.5", () => {
+ shouldBecomeDifferent("muteButton.getBoundingClientRect().width", "0", () => {
+ const muteButtonBounds = muteButton.getBoundingClientRect();
+ // Controls are now visible, let's hover over the mute button to make the volume control visible.
+ eventSender.mouseMoveTo(muteButtonBounds.left + muteButtonBounds.width / 2, muteButtonBounds.top + muteButtonBounds.height / 2);
+ shouldBecomeDifferent("volumeSlider.getBoundingClientRect().width", "0", () => {
+ dragVolumeSlider(0.5, 0);
+ shouldBecomeEqual("media.muted", "true", () => {
+ debug("");
+ debug("We click on the mute button which should set the volume back to 0.5");
+ pressOnElement(muteButton);
+ shouldBecomeEqual("media.volume", "0.5", () => {
+ shouldBeFalse("media.muted");
+ debug("");
+ container.remove();
+ media.remove();
+ finishJSTest();
+ });
+ });
+ });
+ });
+ });
+}
+
+media.addEventListener("play", runTest);
+
+function dragVolumeSlider(fromVolume, toVolume)
+{
+ debug("");
+ debug(`We drag the volume slider to set the volume from ${fromVolume} to ${toVolume}`);
+
+ const bounds = volumeSlider.getBoundingClientRect();
+ const centerX = bounds.left + bounds.width / 2;
+ const dragStartY = bounds.top + bounds.height * (1 - fromVolume);
+ const dragEndY = bounds.top + bounds.height * (1 - toVolume);
+ const delta = dragEndY - dragStartY;
+ const iterations = Math.abs(delta);
+
+ eventSender.mouseMoveTo(centerX, dragStartY);
+ eventSender.mouseDown();
+ for (let i = 1; i <= iterations; ++i)
+ eventSender.mouseMoveTo(dragVolumeSlider, dragStartY + i * Math.sign(delta));
+ eventSender.mouseUp();
+}
+
+</script>
+<script src=""
+</body>
Modified: trunk/Source/WebCore/ChangeLog (213847 => 213848)
--- trunk/Source/WebCore/ChangeLog 2017-03-13 16:29:22 UTC (rev 213847)
+++ trunk/Source/WebCore/ChangeLog 2017-03-13 16:39:08 UTC (rev 213848)
@@ -1,3 +1,41 @@
+2017-03-13 Antoine Quint <[email protected]>
+
+ [Modern Media Controls] Volume icon doesn't turn to mute when the knob is set to 0
+ https://bugs.webkit.org/show_bug.cgi?id=169553
+ <rdar://problem/30754543>
+
+ Reviewed by Eric Carlson.
+
+ When we start changing the volume using the mouse, we record the volume at that point
+ and as we drag the volume slider, we set the mute button to show that the media is
+ muted if the volume is 0, and set the actual media volume to be the initial volume
+ prior to dragging. This way, when we toggle the "muted" property by pressing the
+ mute button, the original volume is restored.
+
+ To function correctly, this required some changed to Slider. The volume slider would
+ fail to update if the value set was the same as the initial value since we would use
+ only the "change" event to identify the end of a slider drag interaction. This was
+ incorrect since if the initial value and the final value while dragging the slider
+ were the same, no "change" event would be fired. So we now use mouse events to
+ identify when the slider is being dragged.
+
+ Test: media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
+
+ * Modules/modern-media-controls/controls/slider.js:
+ (Slider.prototype.set value):
+ (Slider.prototype.handleEvent):
+ (Slider.prototype._handleMousedownEvent):
+ (Slider.prototype._handleInputEvent):
+ (Slider.prototype._handleMouseupEvent):
+ (Slider.prototype._handleChangeEvent): Deleted.
+ * Modules/modern-media-controls/controls/volume-slider.js:
+ (VolumeSlider):
+ (VolumeSlider.prototype.draw):
+ (VolumeSlider.prototype.handleEvent): Deleted.
+ * Modules/modern-media-controls/media/volume-support.js:
+ (VolumeSupport.prototype.controlValueWillStartChanging):
+ (VolumeSupport.prototype.controlValueDidChange):
+
2017-03-13 Per Arne Vollan <[email protected]>
[Win] Compile error.
Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/slider.js (213847 => 213848)
--- trunk/Source/WebCore/Modules/modern-media-controls/controls/slider.js 2017-03-13 16:29:22 UTC (rev 213847)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/slider.js 2017-03-13 16:39:08 UTC (rev 213848)
@@ -36,9 +36,10 @@
this._canvas = new LayoutNode(`<canvas></canvas>`);
this._input = new LayoutNode(`<input type="range" min="0" max="1" step="0.001" />`);
- this._input.element.addEventListener("change", this);
+ this._input.element.addEventListener("mousedown", this);
this._input.element.addEventListener("input", this);
+ this.isActive = false;
this.value = 0;
this.children = [this._canvas, this._input];
@@ -55,7 +56,7 @@
set value(value)
{
- if (this._valueIsChanging)
+ if (this.isActive)
return;
this._value = value;
@@ -79,12 +80,15 @@
handleEvent(event)
{
switch (event.type) {
+ case "mousedown":
+ this._handleMousedownEvent();
+ break;
+ case "mouseup":
+ this._handleMouseupEvent();
+ break;
case "input":
this._handleInputEvent();
break;
- case "change":
- this._handleChangeEvent();
- break;
}
}
@@ -118,11 +122,20 @@
// Private
+ _handleMousedownEvent()
+ {
+ const mediaControls = this.parentOfType(MediaControls);
+ this._mouseupTarget = (!mediaControls || mediaControls instanceof MacOSInlineMediaControls) ? window : mediaControls.element;
+ this._mouseupTarget.addEventListener("mouseup", this, true);
+
+ if (this.uiDelegate && typeof this.uiDelegate.controlValueWillStartChanging === "function")
+ this.uiDelegate.controlValueWillStartChanging(this);
+ this.isActive = true;
+ this.needsLayout = true;
+ }
+
_handleInputEvent()
{
- if (!this._valueIsChanging && this.uiDelegate && typeof this.uiDelegate.controlValueWillStartChanging === "function")
- this.uiDelegate.controlValueWillStartChanging(this);
- this._valueIsChanging = true;
if (this.uiDelegate && typeof this.uiDelegate.controlValueDidChange === "function")
this.uiDelegate.controlValueDidChange(this);
@@ -129,9 +142,12 @@
this.needsLayout = true;
}
- _handleChangeEvent()
+ _handleMouseupEvent()
{
- delete this._valueIsChanging;
+ this._mouseupTarget.removeEventListener("mouseup", this, true);
+ delete this._mouseupTarget;
+
+ this.isActive = false;
if (this.uiDelegate && typeof this.uiDelegate.controlValueDidStopChanging === "function")
this.uiDelegate.controlValueDidStopChanging(this);
Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/volume-slider.js (213847 => 213848)
--- trunk/Source/WebCore/Modules/modern-media-controls/controls/volume-slider.js 2017-03-13 16:29:22 UTC (rev 213847)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/volume-slider.js 2017-03-13 16:39:08 UTC (rev 213848)
@@ -35,26 +35,10 @@
this.height = 11;
this.enabled = true;
-
- this._active = false;
- this.element.addEventListener("mousedown", this);
}
// Protected
- handleEvent(event)
- {
- super.handleEvent(event);
-
- if (event instanceof MouseEvent) {
- this._active = event.type === "mousedown";
- if (event.type === "mousedown")
- window.addEventListener("mouseup", this, true);
- else
- window.removeEventListener("mouseup", this, true);
- }
- }
-
draw(ctx)
{
const width = this.width;
@@ -110,7 +94,7 @@
addRoundedRect(ctx, knobX + 1, 0, knobDiameter, knobDiameter, knobRadius);
ctx.closePath();
ctx.clip();
- ctx.fillStyle = this._active ? "white" : "rgb(138, 138, 138)";
+ ctx.fillStyle = this.isActive ? "white" : "rgb(138, 138, 138)";
ctx.fillRect(0, 0, width, height);
ctx.restore();
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/volume-support.js (213847 => 213848)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/volume-support.js 2017-03-13 16:29:22 UTC (rev 213847)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/volume-support.js 2017-03-13 16:39:08 UTC (rev 213848)
@@ -40,12 +40,14 @@
controlValueWillStartChanging(control)
{
+ this._volumeBeforeChange = this.mediaController.media.volume;
this.mediaController.media.muted = false;
}
controlValueDidChange(control)
{
- this.mediaController.media.volume = control.value;
+ this.mediaController.media.volume = (control.value === 0 && this._volumeBeforeChange > 0) ? this._volumeBeforeChange : control.value;
+ this.mediaController.media.muted = control.value === 0;
}
syncControl()
