Changeset 213848 in webkit


Ignore:
Timestamp:
Mar 13, 2017 9:39:08 AM (7 years ago)
Author:
commit-queue@webkit.org
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 <Antoine Quint> 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.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r213831 r213848  
     12017-03-13  Antoine Quint  <graouts@apple.com>
     2
     3        [Modern Media Controls] Volume icon doesn't turn to mute when the knob is set to 0
     4        https://bugs.webkit.org/show_bug.cgi?id=169553
     5        <rdar://problem/30754543>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Adding a new test where we drag the volume slider to 0 and ensure that the volume gets muted
     10        and that clicking on the mute button resets the volume to be the same value as prior to the
     11        dragging interaction.
     12
     13        * media/modern-media-controls/volume-support/volume-support-drag-to-mute-expected.txt: Added.
     14        * media/modern-media-controls/volume-support/volume-support-drag-to-mute.html: Added.
     15
    1162017-03-13  Manuel Rego Casasnovas  <rego@igalia.com>
    217
  • trunk/Source/WebCore/ChangeLog

    r213847 r213848  
     12017-03-13  Antoine Quint  <graouts@apple.com>
     2
     3        [Modern Media Controls] Volume icon doesn't turn to mute when the knob is set to 0
     4        https://bugs.webkit.org/show_bug.cgi?id=169553
     5        <rdar://problem/30754543>
     6
     7        Reviewed by Eric Carlson.
     8
     9        When we start changing the volume using the mouse, we record the volume at that point
     10        and as we drag the volume slider, we set the mute button to show that the media is
     11        muted if the volume is 0, and set the actual media volume to be the initial volume
     12        prior to dragging. This way, when we toggle the "muted" property by pressing the
     13        mute button, the original volume is restored.
     14
     15        To function correctly, this required some changed to Slider. The volume slider would
     16        fail to update if the value set was the same as the initial value since we would use
     17        only the "change" event to identify the end of a slider drag interaction. This was
     18        incorrect since if the initial value and the final value while dragging the slider
     19        were the same, no "change" event would be fired. So we now use mouse events to
     20        identify when the slider is being dragged.
     21
     22        Test: media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
     23
     24        * Modules/modern-media-controls/controls/slider.js:
     25        (Slider.prototype.set value):
     26        (Slider.prototype.handleEvent):
     27        (Slider.prototype._handleMousedownEvent):
     28        (Slider.prototype._handleInputEvent):
     29        (Slider.prototype._handleMouseupEvent):
     30        (Slider.prototype._handleChangeEvent): Deleted.
     31        * Modules/modern-media-controls/controls/volume-slider.js:
     32        (VolumeSlider):
     33        (VolumeSlider.prototype.draw):
     34        (VolumeSlider.prototype.handleEvent): Deleted.
     35        * Modules/modern-media-controls/media/volume-support.js:
     36        (VolumeSupport.prototype.controlValueWillStartChanging):
     37        (VolumeSupport.prototype.controlValueDidChange):
     38
    1392017-03-13  Per Arne Vollan  <pvollan@apple.com>
    240
  • trunk/Source/WebCore/Modules/modern-media-controls/controls/slider.js

    r212280 r213848  
    3737
    3838        this._input = new LayoutNode(`<input type="range" min="0" max="1" step="0.001" />`);
    39         this._input.element.addEventListener("change", this);
     39        this._input.element.addEventListener("mousedown", this);
    4040        this._input.element.addEventListener("input", this);
    4141
     42        this.isActive = false;
    4243        this.value = 0;
    4344
     
    5657    set value(value)
    5758    {
    58         if (this._valueIsChanging)
     59        if (this.isActive)
    5960            return;
    6061
     
    8081    {
    8182        switch (event.type) {
     83        case "mousedown":
     84            this._handleMousedownEvent();
     85            break;
     86        case "mouseup":
     87            this._handleMouseupEvent();
     88            break;
    8289        case "input":
    8390            this._handleInputEvent();
    84             break;
    85         case "change":
    86             this._handleChangeEvent();
    8791            break;
    8892        }
     
    119123    // Private
    120124
     125    _handleMousedownEvent()
     126    {
     127        const mediaControls = this.parentOfType(MediaControls);
     128        this._mouseupTarget = (!mediaControls || mediaControls instanceof MacOSInlineMediaControls) ? window : mediaControls.element;
     129        this._mouseupTarget.addEventListener("mouseup", this, true);
     130
     131        if (this.uiDelegate && typeof this.uiDelegate.controlValueWillStartChanging === "function")
     132            this.uiDelegate.controlValueWillStartChanging(this);
     133        this.isActive = true;
     134        this.needsLayout = true;
     135    }
     136
    121137    _handleInputEvent()
    122138    {
    123         if (!this._valueIsChanging && this.uiDelegate && typeof this.uiDelegate.controlValueWillStartChanging === "function")
    124             this.uiDelegate.controlValueWillStartChanging(this);
    125         this._valueIsChanging = true;
    126139        if (this.uiDelegate && typeof this.uiDelegate.controlValueDidChange === "function")
    127140            this.uiDelegate.controlValueDidChange(this);
     
    130143    }
    131144
    132     _handleChangeEvent()
     145    _handleMouseupEvent()
    133146    {
    134         delete this._valueIsChanging;
     147        this._mouseupTarget.removeEventListener("mouseup", this, true);
     148        delete this._mouseupTarget;
     149
     150        this.isActive = false;
    135151        if (this.uiDelegate && typeof this.uiDelegate.controlValueDidStopChanging === "function")
    136152            this.uiDelegate.controlValueDidStopChanging(this);
  • trunk/Source/WebCore/Modules/modern-media-controls/controls/volume-slider.js

    r212280 r213848  
    3636        this.height = 11;
    3737        this.enabled = true;
    38 
    39         this._active = false;
    40         this.element.addEventListener("mousedown", this);
    4138    }
    4239
    4340    // Protected
    44 
    45     handleEvent(event)
    46     {
    47         super.handleEvent(event);
    48 
    49         if (event instanceof MouseEvent) {
    50             this._active = event.type === "mousedown";
    51             if (event.type === "mousedown")
    52                 window.addEventListener("mouseup", this, true);
    53             else
    54                 window.removeEventListener("mouseup", this, true);
    55         }
    56     }
    5741
    5842    draw(ctx)
     
    11195        ctx.closePath();
    11296        ctx.clip();
    113         ctx.fillStyle = this._active ? "white" : "rgb(138, 138, 138)";
     97        ctx.fillStyle = this.isActive ? "white" : "rgb(138, 138, 138)";
    11498        ctx.fillRect(0, 0, width, height);
    11599        ctx.restore();
  • trunk/Source/WebCore/Modules/modern-media-controls/media/volume-support.js

    r208080 r213848  
    4141    controlValueWillStartChanging(control)
    4242    {
     43        this._volumeBeforeChange = this.mediaController.media.volume;
    4344        this.mediaController.media.muted = false;
    4445    }
     
    4647    controlValueDidChange(control)
    4748    {
    48         this.mediaController.media.volume = control.value;
     49        this.mediaController.media.volume = (control.value === 0 && this._volumeBeforeChange > 0) ? this._volumeBeforeChange : control.value;
     50        this.mediaController.media.muted = control.value === 0;
    4951    }
    5052
Note: See TracChangeset for help on using the changeset viewer.