From 4e78bb3914f076014281b9b23410b1a37bb4dd27 Mon Sep 17 00:00:00 2001 From: "jer.noble@apple.com" Date: Tue, 21 Oct 2014 16:28:34 +0000 Subject: [PATCH] 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. 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 | 10 ++++ .../media/video-volume-slider-drag-expected.txt | 8 +++ LayoutTests/media/video-volume-slider-drag.html | 59 ++++++++++++++++++++++ Source/WebCore/ChangeLog | 25 +++++++++ .../Modules/mediacontrols/mediaControlsApple.js | 17 ++++--- 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 LayoutTests/media/video-volume-slider-drag-expected.txt create mode 100644 LayoutTests/media/video-volume-slider-drag.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 740008b..7d1fe66 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2014-10-21 Jer Noble + + 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 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 index 0000000..9c7aa04 --- /dev/null +++ b/LayoutTests/media/video-volume-slider-drag-expected.txt @@ -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 index 0000000..eea3553 --- /dev/null +++ b/LayoutTests/media/video-volume-slider-drag.html @@ -0,0 +1,59 @@ + + + Test that dragging the volume slider results in multiple volume change events. + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 276a54b..bdcdca4 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,28 @@ +2014-10-21 Jer Noble + + 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 ASSERTION FAILED: !gridWasPopulated() in WebCore::RenderGrid::placeItemsOnGrid diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js index 30c18f4..874cfb4 100644 --- a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js +++ b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js @@ -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; -- 1.8.3.1