Changeset 274293 in webkit


Ignore:
Timestamp:
Mar 11, 2021 12:33:42 PM (17 months ago)
Author:
Devin Rousso
Message:

Video controls stay on screen indefinitely after interacting with time scrubber
​https://bugs.webkit.org/show_bug.cgi?id=223081
<rdar://problem/73935705>

Reviewed by Eric Carlson.

Source/WebCore:

Test: media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading.html

  • Modules/modern-media-controls/controls/auto-hide-controller.js:

(AutoHideController):
(AutoHideController.prototype.set fadesWhileIdle):
(AutoHideController.prototype.get hasSecondaryUIAttached): Added.
(AutoHideController.prototype.set hasSecondaryUIAttached): Added.
(AutoHideController.prototype.handleEvent):
(AutoHideController.prototype.mediaControlsFadedStateDidChange):
(AutoHideController.prototype.mediaControlsBecameInvisible):
(AutoHideController.prototype.get _canFadeControls): Added.
(AutoHideController.prototype._resetAutoHideTimer):
(AutoHideController.prototype._autoHideTimerFired):
(AutoHideController.prototype._cancelNonEnforcedAutoHideTimer): Deleted.
Simplify the logic of AutoHideController to always listen for "pointer*" events instead
of adding/removing them depending on set fadesWhileIdle. This was problematic because when
the media is paused, ControlsVisibilitySupport will disable fadesWhileIdle, removing all
"pointer*" event listeners, only to re-add them as soon as the media is resumed (this is
especially bad when clicking on the time scrubber track to seek, as this will pause and then
resume the media in rapid succession). Remove _enforceAutoHideTimer as there's no case
where we wouldn't want to be able to delay/cancel the auto-hide based on user interaction.

  • Modules/modern-media-controls/controls/media-controls.js:

(MediaControls.prototype.hideTracksPanel):
Now that set hasSecondaryUIAttached also calls _resetAutoHideTimer, don't manually set
this.faded and instead use the logic/timing in AutoHideController.

LayoutTests:

  • media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading.html:
  • media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading-expected.txt:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r274292 r274293  
     12021-03-11  Devin Rousso  <drousso@apple.com>
     2
     3        Video controls stay on screen indefinitely after interacting with time scrubber
     4        https://bugs.webkit.org/show_bug.cgi?id=223081
     5        <rdar://problem/73935705>
     6
     7        Reviewed by Eric Carlson.
     8
     9        * media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading.html:
     10        * media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading-expected.txt:
     11
    1122021-03-11  Eric Carlson  <eric.carlson@apple.com>
    213

  • trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading-expected.txt

    r228010 r274293  
    1414
    1515mediaControls.hideTracksPanel()
    16 PASS mediaControls.faded is true
     16PASS mediaControls.faded became true
    1717
    1818PASS successfullyParsed is true

  • trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading.html

    r228010 r274293  
    4848        debug("mediaControls.hideTracksPanel()");
    4949        mediaControls.hideTracksPanel();
    50         shouldBeTrue("mediaControls.faded");
    51 
    52         debug("");
    53         mediaControls.element.remove();
    54         finishJSTest();
     50        shouldBecomeEqual("mediaControls.faded", "true", () => {
     51            debug("");
     52            mediaControls.element.remove();
     53            finishJSTest();
     54        });
    5555    }, mediaControls.autoHideController.autoHideDelay);
    5656});

  • trunk/Source/WebCore/ChangeLog

    r274287 r274293  
     12021-03-11  Devin Rousso  <drousso@apple.com>
     2
     3        Video controls stay on screen indefinitely after interacting with time scrubber
     4        https://bugs.webkit.org/show_bug.cgi?id=223081
     5        <rdar://problem/73935705>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Test: media/modern-media-controls/tracks-panel/tracks-panel-prevent-controls-bar-from-fading.html
     10
     11        * Modules/modern-media-controls/controls/auto-hide-controller.js:
     12        (AutoHideController):
     13        (AutoHideController.prototype.set fadesWhileIdle):
     14        (AutoHideController.prototype.get hasSecondaryUIAttached): Added.
     15        (AutoHideController.prototype.set hasSecondaryUIAttached): Added.
     16        (AutoHideController.prototype.handleEvent):
     17        (AutoHideController.prototype.mediaControlsFadedStateDidChange):
     18        (AutoHideController.prototype.mediaControlsBecameInvisible):
     19        (AutoHideController.prototype.get _canFadeControls): Added.
     20        (AutoHideController.prototype._resetAutoHideTimer):
     21        (AutoHideController.prototype._autoHideTimerFired):
     22        (AutoHideController.prototype._cancelNonEnforcedAutoHideTimer): Deleted.
     23        Simplify the logic of `AutoHideController` to always listen for `"pointer*"` events instead
     24        of adding/removing them depending on `set fadesWhileIdle`. This was problematic because when
     25        the media is paused, `ControlsVisibilitySupport` will disable `fadesWhileIdle`, removing all
     26        `"pointer*"` event listeners, only to re-add them as soon as the media is resumed (this is
     27        especially bad when clicking on the time scrubber track to seek, as this will pause and then
     28        resume the media in rapid succession). Remove `_enforceAutoHideTimer` as there's no case
     29        where we wouldn't want to be able to delay/cancel the auto-hide based on user interaction.
     30
     31        * Modules/modern-media-controls/controls/media-controls.js:
     32        (MediaControls.prototype.hideTracksPanel):
     33        Now that `set hasSecondaryUIAttached` also calls `_resetAutoHideTimer`, don't manually set
     34        `this.faded` and instead use the logic/timing in `AutoHideController`.
     35
    1362021-03-11  Rob Buis  <rbuis@igalia.com>
    237

  • trunk/Source/WebCore/Modules/modern-media-controls/controls/auto-hide-controller.js

    r253248 r274293  
    3636        this._pointerIdentifiersPreventingAutoHideForHover = new Set;
    3737
     38        this._mediaControls.element.addEventListener("pointermove", this);
     39        this._mediaControls.element.addEventListener("pointerdown", this);
     40        this._mediaControls.element.addEventListener("pointerup", this);
     41        this._mediaControls.element.addEventListener("pointerleave", this);
     42        this._mediaControls.element.addEventListener("pointerout", this);
     43
    3844        if (GestureRecognizer.SupportsTouches)
    3945            this._tapGestureRecognizer = new TapGestureRecognizer(this._mediaControls.element, this);
    … …  
    5662        this._fadesWhileIdle = flag;
    5763
    58         if (flag) {
    59             this._mediaControls.element.addEventListener("pointermove", this);
    60             this._mediaControls.element.addEventListener("pointerdown", this);
    61             this._mediaControls.element.addEventListener("pointerup", this);
    62             this._mediaControls.element.addEventListener("pointerleave", this);
    63             this._mediaControls.element.addEventListener("pointerout", this);
    64         } else {
    65             this._mediaControls.element.removeEventListener("pointermove", this);
    66             this._mediaControls.element.removeEventListener("pointerdown", this);
    67             this._mediaControls.element.removeEventListener("pointerup", this);
    68             this._mediaControls.element.removeEventListener("pointerleave", this);
    69             this._mediaControls.element.removeEventListener("pointerout", this);
    70         }
     64        if (!this._fadesWhileIdle)
     65            this._mediaControls.faded = false;
    7166
    72         if (flag && !this._mediaControls.faded)
    73             this._resetAutoHideTimer(false);
    74         else if (!flag)
    75             this._mediaControls.faded = false;
     67        this._resetAutoHideTimer();
     68    }
     69
     70    get hasSecondaryUIAttached()
     71    {
     72        return this._hasSecondaryUIAttached;
     73    }
     74
     75    set hasSecondaryUIAttached(flag)
     76    {
     77        if (this._hasSecondaryUIAttached == flag)
     78            return;
     79
     80        this._hasSecondaryUIAttached = flag;
     81
     82        this._resetAutoHideTimer();
    7683    }
    7784
    … …  
    8390            return;
    8491
    85         if (event.type === "pointermove") {
     92        switch (event.type) {
     93        case "pointermove":
    8694            this._mediaControls.faded = false;
    87             this._resetAutoHideTimer(true);
    8895            if (this._mediaControls.isPointInControls(new DOMPoint(event.clientX, event.clientY))) {
    8996                this._pointerIdentifiersPreventingAutoHideForHover.add(event.pointerId);
    90                 this._cancelNonEnforcedAutoHideTimer();
     97                this._cancelAutoHideTimer();
    9198            } else {
    9299                this._pointerIdentifiersPreventingAutoHideForHover.delete(event.pointerId);
    93                 this._resetAutoHideTimer(true);
     100                this._resetAutoHideTimer();
    94101            }
    95         } else if (event.type === "pointerleave" && this._fadesWhileIdle && !this.hasSecondaryUIAttached && !this._enforceAutoHideTimer) {
     102            return;
     103
     104        case "pointerleave":
    96105            this._pointerIdentifiersPreventingAutoHide.delete(event.pointerId);
    97106            this._pointerIdentifiersPreventingAutoHideForHover.delete(event.pointerId);
    … …  
    102111                this._autoHideTimerFired();
    103112
    104             this._resetAutoHideTimer(true);
    105         }
     113            this._resetAutoHideTimer();
     114            return;
    106115
    107         if (event.type === "pointerdown") {
     116        case "pointerdown":
    108117            // Remember the current faded state so that we can determine,
    109118            // if we recognize a tap, if it should fade the controls out.
    … …  
    111120            this._pointerIdentifiersPreventingAutoHide.add(event.pointerId);
    112121            this._mediaControls.faded = false;
    113             this._cancelNonEnforcedAutoHideTimer();
    114         } else if (event.type === "pointerup") {
     122            this._cancelAutoHideTimer();
     123            return;
     124
     125        case "pointerup":
    115126            this._pointerIdentifiersPreventingAutoHide.delete(event.pointerId);
    116             this._resetAutoHideTimer(true);
     127            this._resetAutoHideTimer();
     128            return;
    117129        }
    118130    }
    … …  
    129141    mediaControlsFadedStateDidChange()
    130142    {
    131         if (this._mediaControls.faded)
    132             delete this._enforceAutoHideTimer;
    133         else
    134             this._resetAutoHideTimer(true);
     143        this._resetAutoHideTimer();
    135144    }
    136145
    137146    mediaControlsBecameInvisible()
    138147    {
    139         this._cancelNonEnforcedAutoHideTimer();
     148        this._cancelAutoHideTimer();
    140149    }
    141150
    142151    // Private
    143152
    144     _cancelNonEnforcedAutoHideTimer()
     153    get _canFadeControls()
    145154    {
    146         if (!this._enforceAutoHideTimer)
    147             this._cancelAutoHideTimer();
     155        return this._fadesWhileIdle && !this._hasSecondaryUIAttached;
    148156    }
    149157
    … …  
    154162    }
    155163
    156     _resetAutoHideTimer(cancelable)
     164    _resetAutoHideTimer()
    157165    {
    158         if (cancelable && this._enforceAutoHideTimer)
    159             return;
    160 
    161166        this._cancelAutoHideTimer();
    162167
    163         if (cancelable)
    164             delete this._enforceAutoHideTimer;
    165         else
    166             this._enforceAutoHideTimer = true;
     168        if (this._mediaControls.faded || !this._canFadeControls)
     169            return;
    167170
    168171        this._autoHideTimer = window.setTimeout(this._autoHideTimerFired.bind(this), this.autoHideDelay);
    … …  
    171174    _autoHideTimerFired()
    172175    {
    173         const disableAutoHiding = this._pointerIdentifiersPreventingAutoHide.size || this._pointerIdentifiersPreventingAutoHideForHover.size;
     176        delete this._autoHideTimer;
    174177
    175         delete this._enforceAutoHideTimer;
    176         if (disableAutoHiding)
    177             return;
    178 
    179         this._cancelAutoHideTimer();
    180         this._mediaControls.faded = this._fadesWhileIdle && !this.hasSecondaryUIAttached;
     178        this._mediaControls.faded = this._canFadeControls && !this._pointerIdentifiersPreventingAutoHide.size && !this._pointerIdentifiersPreventingAutoHideForHover.size;
    181179    }
    182180

  • trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js

    r273004 r274293  
    192192        this.element.classList.remove("shows-tracks-panel");
    193193
    194         let shouldFadeControlsBar = true;
    195         if (window.event instanceof MouseEvent)
    196             shouldFadeControlsBar = !this.isPointInControls(new DOMPoint(event.clientX, event.clientY), true);
    197 
    198194        this.tracksButton.on = false;
    199195        this.tracksButton.element.focus();
    200196        this.autoHideController.hasSecondaryUIAttached = false;
    201         this.faded = this.autoHideController.fadesWhileIdle && shouldFadeControlsBar;
    202197        this.tracksPanel.hide();
    203198    }
Note: See TracChangeset for help on using the changeset viewer.