REGRESSION (r170808): Volume slider in built-in media controls only changes volume...
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Oct 2014 16:28:34 +0000 (16:28 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Oct 2014 16:28:34 +0000 (16:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137805

Reviewed by Dan Bernstein.

Source/WebCore:

Test: media/video-volume-slider-drag.html

Respond to the 'input' event rather than the 'change' event for the volume slider, so that
volume changes are continuous during drag operations.

Also listen for both 'input' and 'change' events for the timeline slider, doing fastSeek()
during 'input' and setting an explicit currentTime during 'change'. This is the same behavior
as current, but using 'change' instead of 'mouseup' to do the final currentTime change.

* Modules/mediacontrols/mediaControlsApple.js:
(Controller.prototype.createControls):
(Controller.prototype.handleTimelineInput):
(Controller.prototype.handleTimelineChange):
(Controller.prototype.handleTimelineMouseUp):
(Controller.prototype.handleVolumeSliderInput):
(Controller.prototype.handlePlayButtonClicked): Deleted.
(Controller.prototype.handleMaxButtonClicked): Deleted.

LayoutTests:

* media/video-volume-slider-drag-expected.txt: Added.
* media/video-volume-slider-drag.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/media/video-volume-slider-drag-expected.txt [new file with mode: 0644]
LayoutTests/media/video-volume-slider-drag.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediacontrols/mediaControlsApple.js

index 740008b..7d1fe66 100644 (file)
@@ -1,3 +1,13 @@
+2014-10-21  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (r170808): Volume slider in built-in media controls only changes volume when thumb is released, not while dragging
+        https://bugs.webkit.org/show_bug.cgi?id=137805
+
+        Reviewed by Dan Bernstein.
+
+        * media/video-volume-slider-drag-expected.txt: Added.
+        * media/video-volume-slider-drag.html: Added.
+
 2014-10-21  Manuel Rego Casasnovas  <rego@igalia.com>
 
         ASSERTION FAILED: !gridWasPopulated() in WebCore::RenderGrid::placeItemsOnGrid
diff --git a/LayoutTests/media/video-volume-slider-drag-expected.txt b/LayoutTests/media/video-volume-slider-drag-expected.txt
new file mode 100644 (file)
index 0000000..9c7aa04
--- /dev/null
@@ -0,0 +1,8 @@
+
+EVENT(canplaythrough)
+EVENT(volumechange)
+EVENT(volumechange)
+EVENT(volumechange)
+EVENT(volumechange)
+END OF TEST
+
diff --git a/LayoutTests/media/video-volume-slider-drag.html b/LayoutTests/media/video-volume-slider-drag.html
new file mode 100644 (file)
index 0000000..eea3553
--- /dev/null
@@ -0,0 +1,59 @@
+<html>
+<head>
+    <title>Test that dragging the volume slider results in multiple volume change events.</title>
+    <script src="media-file.js"></script>
+    <script src="media-controls.js"></script>
+    <script src="video-test.js"></script>
+    <script>
+        var video;
+        var expectedVolumeChangeCount = 4;
+        var volumeChangeCount = 0;
+
+        function init()
+        {
+            if (window.internals)
+                internals.suspendAnimations();
+
+            findMediaElement();
+
+            waitForEvent('canplaythrough', startTest);
+            waitForEvent('volumechange', volumeChange);
+            video.src = findMediaFile('video', 'content/test');
+        }
+
+        function startTest()
+        {
+            if (!window.eventSender)
+                return;
+
+            muteCoords = mediaControlsButtonCoordinates(video, 'mute-button');
+            eventSender.mouseMoveTo(muteCoords[0], muteCoords[1]);
+            setTimeout(move, 100);
+        }
+
+        function move()
+        {
+            if (!window.eventSender)
+                return;
+
+            volumeCoords = mediaControlsButtonCoordinates(video, 'volume-slider');
+            eventSender.mouseMoveTo(volumeCoords[0], volumeCoords[1]);
+            eventSender.mouseDown();
+            for (var i = 10; i < 80; i += 5)
+            eventSender.mouseMoveTo(volumeCoords[0], volumeCoords[1] - i);
+            eventSender.mouseUp();
+
+            failTestIn(1000);
+        }
+
+        function volumeChange()
+        {
+            if (++volumeChangeCount === expectedVolumeChangeCount)
+                endTest();
+        }
+    </script>
+</head>
+<body onload="init()">
+    <video controls></video>
+</body>
+</html>
index 276a54b..bdcdca4 100644 (file)
@@ -1,3 +1,28 @@
+2014-10-21  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (r170808): Volume slider in built-in media controls only changes volume when thumb is released, not while dragging
+        https://bugs.webkit.org/show_bug.cgi?id=137805
+
+        Reviewed by Dan Bernstein.
+
+        Test: media/video-volume-slider-drag.html
+
+        Respond to the 'input' event rather than the 'change' event for the volume slider, so that
+        volume changes are continuous during drag operations.
+
+        Also listen for both 'input' and 'change' events for the timeline slider, doing fastSeek()
+        during 'input' and setting an explicit currentTime during 'change'. This is the same behavior
+        as current, but using 'change' instead of 'mouseup' to do the final currentTime change.
+
+        * Modules/mediacontrols/mediaControlsApple.js:
+        (Controller.prototype.createControls):
+        (Controller.prototype.handleTimelineInput):
+        (Controller.prototype.handleTimelineChange):
+        (Controller.prototype.handleTimelineMouseUp):
+        (Controller.prototype.handleVolumeSliderInput):
+        (Controller.prototype.handlePlayButtonClicked): Deleted.
+        (Controller.prototype.handleMaxButtonClicked): Deleted.
+
 2014-10-21  Manuel Rego Casasnovas  <rego@igalia.com>
 
         ASSERTION FAILED: !gridWasPopulated() in WebCore::RenderGrid::placeItemsOnGrid
index 30c18f4..874cfb4 100644 (file)
@@ -334,7 +334,8 @@ Controller.prototype = {
         timeline.setAttribute('aria-label', this.UIString('Duration'));
         timeline.type = 'range';
         timeline.value = 0;
-        this.listenFor(timeline, 'input', this.handleTimelineChange);
+        this.listenFor(timeline, 'input', this.handleTimelineInput);
+        this.listenFor(timeline, 'change', this.handleTimelineChange);
         this.listenFor(timeline, 'mouseover', this.handleTimelineMouseOver);
         this.listenFor(timeline, 'mouseout', this.handleTimelineMouseOut);
         this.listenFor(timeline, 'mousemove', this.handleTimelineMouseMove);
@@ -385,7 +386,7 @@ Controller.prototype = {
         volume.min = 0;
         volume.max = 1;
         volume.step = .01;
-        this.listenFor(volume, 'change', this.handleVolumeSliderChange);
+        this.listenFor(volume, 'input', this.handleVolumeSliderInput);
 
         var captionButton = this.controls.captionButton = document.createElement('button');
         captionButton.setAttribute('pseudo', '-webkit-media-controls-toggle-closed-captions-button');
@@ -740,11 +741,16 @@ Controller.prototype = {
         return true;
     },
 
-    handleTimelineChange: function(event)
+    handleTimelineInput: function(event)
     {
         this.video.fastSeek(this.controls.timeline.value);
     },
 
+    handleTimelineChange: function(event)
+    {
+        this.video.currentTime = this.controls.timeline.value;
+    },
+
     handleTimelineDown: function(event)
     {
         this.controls.thumbnail.classList.add(this.ClassNames.show);
@@ -804,9 +810,6 @@ Controller.prototype = {
     handleTimelineMouseUp: function(event)
     {
         this.scrubbing = false;
-
-        // Do a precise seek when we lift the mouse:
-        this.video.currentTime = this.controls.timeline.value;
     },
 
     handleMuteButtonClicked: function(event)
@@ -836,7 +839,7 @@ Controller.prototype = {
         this.video.volume = 1;
     },
 
-    handleVolumeSliderChange: function(event)
+    handleVolumeSliderInput: function(event)
     {
         if (this.video.muted) {
             this.video.muted = false;