Changeset 228445 in webkit


Ignore:
Timestamp:
Feb 13, 2018 5:36:22 PM (6 years ago)
Author:
graouts@webkit.org
Message:

Removing the controls attribute from a <video> element does not tear down the controls shadow DOM nor cancel event listeners.
​https://bugs.webkit.org/show_bug.cgi?id=182668
Source/WebCore:

<rdar://problem/33793004>

Reviewed by Jer Noble.

When controls were turned off for inline media players, we would remove all media controls elements from the shadow root,
but we would nevertheless continue to listen to media events and, as a result, update properties of the media controls
which would lead to requestAnimationFrame() calls that would update the detached DOM nodes.

We now only listent to media events if controls are turned on.

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

(ControlsVisibilitySupport.prototype.enable): Remove the mutation observer from ControlsVisibilitySupport since observing
changes to the controls attribute is now performed directly in MediaController. We need to make sure that we update the
controls however since fadesWhileIdle is turned off in the disable() call to ensure that the auto-hide behavior is disabled
as well.
(ControlsVisibilitySupport.prototype.disable): Disable the auto-hide controller as well.
(ControlsVisibilitySupport.prototype._updateControls): Remove code that has now been moved into MediaController._updateControlsAvailability().

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

(MediaController): Listen to the "play" event on the media so that we call _updateControlsAvailability() in this situation to account for
shouldForceControlsDisplay on MediaControlsHost. We also register for a mutation observer to track when the controls attribute availability
changes in which case we want to call _updateControlsAvailability() as well.
(MediaController.prototype.handleEvent): Call _updateControlsAvailability() instead of _updateiOSFullscreenProperties() which has been renamed
and expanded.
(MediaController.prototype._updateControlsIfNeeded): Call _updateControlsAvailability() after controls have been updated.
(MediaController.prototype._updateControlsAvailability): We now disable supporting media controller objects when we know that controls should
be hidden in all cases except when in fullscreen on macOS.
(MediaController.prototype._updateiOSFullscreenProperties): Deleted.

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

(PlacardSupport.prototype.disable): Only allow the media events required to track when to show placards when in fullscreen since inline media
players need to show the AirPlay and picture-in-picture placards even when controls are disabled.

LayoutTests:

Reviewed by Jer Noble.

Ensure controls are turned on for a number of tests that would fail otherwise since media events would not be handled by media
controls without it.

  • http/tests/media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-live-broadcast.html:
  • http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html:
  • http/tests/media/modern-media-controls/skip-back-support/skip-back-support-button-click.html:
  • http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html:
  • http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html:
  • http/tests/media/modern-media-controls/status-support/status-support-loading.html:
  • media/modern-media-controls/airplay-support/airplay-support.html:
  • media/modern-media-controls/mute-support/mute-support-media-api.html:
  • media/modern-media-controls/playback-support/playback-support-autoplay.html:
  • media/modern-media-controls/playback-support/playback-support-media-api.html:
  • media/modern-media-controls/start-support/start-support-error.html:
  • media/modern-media-controls/start-support/start-support-lowPowerMode.html:
  • media/modern-media-controls/start-support/start-support-manual-play.html:
  • media/modern-media-controls/status-support/status-support-error.html:
  • media/modern-media-controls/time-labels-support/elapsed-time.html:
  • media/modern-media-controls/time-labels-support/remaining-time.html:
  • media/modern-media-controls/tracks-support/tracks-support-audio-tracks.html:
  • media/modern-media-controls/tracks-support/tracks-support-text-tracks.html:
  • media/modern-media-controls/volume-support/volume-support-media-api-mute.html:
  • media/modern-media-controls/volume-support/volume-support-media-api.html:
Location:
trunk
Files:
25 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r228444 r228445  
     12018-02-13  Antoine Quint  <graouts@apple.com>
     2
     3        Removing the controls attribute from a <video> element does not tear down the controls shadow DOM nor cancel event listeners.
     4        https://bugs.webkit.org/show_bug.cgi?id=182668
     5
     6        Reviewed by Jer Noble.
     7
     8        Ensure controls are turned on for a number of tests that would fail otherwise since media events would not be handled by media
     9        controls without it.
     10
     11        * http/tests/media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-live-broadcast.html:
     12        * http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html:
     13        * http/tests/media/modern-media-controls/skip-back-support/skip-back-support-button-click.html:
     14        * http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html:
     15        * http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html:
     16        * http/tests/media/modern-media-controls/status-support/status-support-loading.html:
     17        * media/modern-media-controls/airplay-support/airplay-support.html:
     18        * media/modern-media-controls/mute-support/mute-support-media-api.html:
     19        * media/modern-media-controls/playback-support/playback-support-autoplay.html:
     20        * media/modern-media-controls/playback-support/playback-support-media-api.html:
     21        * media/modern-media-controls/start-support/start-support-error.html:
     22        * media/modern-media-controls/start-support/start-support-lowPowerMode.html:
     23        * media/modern-media-controls/start-support/start-support-manual-play.html:
     24        * media/modern-media-controls/status-support/status-support-error.html:
     25        * media/modern-media-controls/time-labels-support/elapsed-time.html:
     26        * media/modern-media-controls/time-labels-support/remaining-time.html:
     27        * media/modern-media-controls/tracks-support/tracks-support-audio-tracks.html:
     28        * media/modern-media-controls/tracks-support/tracks-support-text-tracks.html:
     29        * media/modern-media-controls/volume-support/volume-support-media-api-mute.html:
     30        * media/modern-media-controls/volume-support/volume-support-media-api.html:
     31
    1322018-02-13  Jiewen Tan  <jiewen_tan@apple.com>
    233

  • trunk/LayoutTests/http/tests/media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-live-broadcast.html

    r228097 r228445  
    33<script src="/media-resources/modern-media-controls/resources/media-controls-utils.js"></script>
    44<body>
    5 <video src="../../resources/hls/test-live.php" style="width: 400px; height: 300px;" autoplay></video>
     5<video src="../../resources/hls/test-live.php" style="width: 400px; height: 300px;" autoplay controls></video>
    66<div id="shadow"></div>
    77<script type="text/javascript">

  • trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html

    r211687 r228445  
    22<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
    33<body>
    4 <video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay></video>
     4<video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/http/tests/media/modern-media-controls/skip-back-support/skip-back-support-button-click.html

    r228097 r228445  
    22<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
    33<body>
    4 <video src="../../resources/hls/generate-vod.php?duration=8000" style="width: 320px; height: 240px;"></video>
     4<video src="../../resources/hls/generate-vod.php?duration=8000" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html

    r211641 r228445  
    22<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
    33<body>
    4 <video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay></video>
     4<video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html

    r208491 r228445  
    22<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
    33<body>
    4 <video src="../../resources/hls/test-live.php" style="width: 320px; height: 240px;" autoplay></video>
     4<video src="../../resources/hls/test-live.php" style="width: 320px; height: 240px;" autoplay controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/http/tests/media/modern-media-controls/status-support/status-support-loading.html

    r228097 r228445  
    22<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
    33<body>
    4 <video src="http://127.0.0.1:8000/resources/load-and-stall.cgi?name=../../../media/content/test.mp4&mimeType=video/mp4&stallAt=1&stallFor=4" style="width: 320px; height: 240px;"></video>
     4<video src="http://127.0.0.1:8000/resources/load-and-stall.cgi?name=../../../media/content/test.mp4&mimeType=video/mp4&stallAt=1&stallFor=4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/airplay-support/airplay-support.html

    r219412 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;" autoplay></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" autoplay controls></video>
    55<div id="host"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/mute-support/mute-support-media-api.html

    r208226 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/playback-support/playback-support-autoplay.html

    r208226 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;" autoplay></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" autoplay controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/playback-support/playback-support-media-api.html

    r215916 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/start-support/start-support-error.html

    r208226 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="404.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="404.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/start-support/start-support-lowPowerMode.html

    r227904 r228445  
    4646    }
    4747});
     48
     49setTimeout(finishJSTest, 5000);
     50
    4851</script>
    4952<script src="../../../resources/js-test-post.js"></script>

  • trunk/LayoutTests/media/modern-media-controls/start-support/start-support-manual-play.html

    r215916 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/status-support/status-support-error.html

    r208491 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="404.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="404.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/time-labels-support/elapsed-time.html

    r219715 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/time-labels-support/remaining-time.html

    r219715 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/tracks-support/tracks-support-audio-tracks.html

    r228010 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/audio-tracks.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/audio-tracks.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/tracks-support/tracks-support-text-tracks.html

    r228010 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/counting-subtitled.m4v" style="width: 320px; height: 240px;" preload="preload"></video>
     4<video src="../../content/counting-subtitled.m4v" style="width: 320px; height: 240px;" preload="preload" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-media-api-mute.html

    r208226 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-media-api.html

    r208226 r228445  
    22<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
    33<body>
    4 <video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
     4<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
    55<div id="shadow"></div>
    66<script type="text/javascript">

  • trunk/Source/WebCore/ChangeLog

    r228444 r228445  
     12018-02-13  Antoine Quint  <graouts@apple.com>
     2
     3        Removing the controls attribute from a <video> element does not tear down the controls shadow DOM nor cancel event listeners.
     4        https://bugs.webkit.org/show_bug.cgi?id=182668
     5        <rdar://problem/33793004>
     6
     7        Reviewed by Jer Noble.
     8
     9        When controls were turned off for inline media players, we would remove all media controls elements from the shadow root,
     10        but we would nevertheless continue to listen to media events and, as a result, update properties of the media controls
     11        which would lead to requestAnimationFrame() calls that would update the detached DOM nodes.
     12
     13        We now only listent to media events if controls are turned on.
     14
     15        * Modules/modern-media-controls/media/controls-visibility-support.js:
     16        (ControlsVisibilitySupport.prototype.enable): Remove the mutation observer from ControlsVisibilitySupport since observing
     17        changes to the controls attribute is now performed directly in MediaController. We need to make sure that we update the
     18        controls however since fadesWhileIdle is turned off in the disable() call to ensure that the auto-hide behavior is disabled
     19        as well.
     20        (ControlsVisibilitySupport.prototype.disable): Disable the auto-hide controller as well.
     21        (ControlsVisibilitySupport.prototype._updateControls): Remove code that has now been moved into MediaController._updateControlsAvailability().
     22        * Modules/modern-media-controls/media/media-controller.js:
     23        (MediaController): Listen to the "play" event on the media so that we call _updateControlsAvailability() in this situation to account for
     24        shouldForceControlsDisplay on MediaControlsHost. We also register for a mutation observer to track when the controls attribute availability
     25        changes in which case we want to call _updateControlsAvailability() as well.
     26        (MediaController.prototype.handleEvent): Call _updateControlsAvailability() instead of _updateiOSFullscreenProperties() which has been renamed
     27        and expanded.
     28        (MediaController.prototype._updateControlsIfNeeded): Call _updateControlsAvailability() after controls have been updated.
     29        (MediaController.prototype._updateControlsAvailability): We now disable supporting media controller objects when we know that controls should
     30        be hidden in all cases except when in fullscreen on macOS.
     31        (MediaController.prototype._updateiOSFullscreenProperties): Deleted.
     32        * Modules/modern-media-controls/media/placard-support.js:
     33        (PlacardSupport.prototype.disable): Only allow the media events required to track when to show placards when in fullscreen since inline media
     34        players need to show the AirPlay and picture-in-picture placards even when controls are disabled.
     35
    1362018-02-13  Jiewen Tan  <jiewen_tan@apple.com>
    237

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

    r225279 r228445  
    3939    {
    4040        super.enable();
    41 
    42         if (this._controlsAttributeObserver)
    43             return;
    44 
    45         this._controlsAttributeObserver = new MutationObserver(this._updateControls.bind(this));
    46         this._controlsAttributeObserver.observe(this.mediaController.media, { attributes: true, attributeFilter: ["controls"] });
     41        this._updateControls();
    4742    }
    4843
    … …  
    5045    {
    5146        super.disable();
    52 
    53         if (!this._controlsAttributeObserver)
    54             return;
    55 
    56         this._controlsAttributeObserver.disconnect();
    57         delete this._controlsAttributeObserver;
     47        this.mediaController.controls.autoHideController.fadesWhileIdle = false;
    5848    }
    5949
    … …  
    7868    {
    7969        const media = this.mediaController.media;
    80         const host = this.mediaController.host;
    81         const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay) || this.mediaController.isFullscreen);
    8270        const isVideo = media instanceof HTMLVideoElement && media.videoTracks.length > 0;
    83 
    84         const controls = this.mediaController.controls;
    85         controls.visible = shouldShowControls;
    86         controls.autoHideController.fadesWhileIdle = isVideo ? !media.paused && !media.webkitCurrentPlaybackTargetIsWireless : false;
     71        this.mediaController.controls.autoHideController.fadesWhileIdle = isVideo ? !media.paused && !media.webkitCurrentPlaybackTargetIsWireless : false;
    8772    }
    8873

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

    r227904 r228445  
    5757        media.videoTracks.addEventListener("removetrack", this);
    5858
     59        media.addEventListener("play", this);
    5960        media.addEventListener(this.fullscreenChangeEventType, this);
    6061
    6162        window.addEventListener("keydown", this);
     63
     64        new MutationObserver(this._updateControlsAvailability.bind(this)).observe(this.media, { attributes: true, attributeFilter: ["controls"] });
    6265    }
    6366
    … …  
    165168        } else if (event.currentTarget === this.media) {
    166169            this._updateControlsIfNeeded();
    167             this._updateiOSFullscreenProperties();
     170            this._updateControlsAvailability();
    168171            if (event.type === "webkitpresentationmodechanged")
    169172                this._returnMediaLayerToInlineIfNeeded();
    … …  
    215218
    216219        this.controls.shouldUseSingleBarLayout = this.controls instanceof InlineMediaControls && this.isYouTubeEmbedWithTitle;
     220
     221        this._updateControlsAvailability();
    217222    }
    218223
    … …  
    301306    }
    302307
    303     _updateiOSFullscreenProperties()
    304     {
    305         // On iOS, we want to make sure not to update controls when we're in fullscreen since the UI
    306         // will be completely invisible.
    307         if (!(this.layoutTraits & LayoutTraits.iOS))
    308             return;
    309 
    310         const isFullscreen = this.isFullscreen;
    311         if (isFullscreen)
     308    _updateControlsAvailability()
     309    {
     310        const shouldControlsBeAvailable = !!(this.media.controls || (this.host && this.host.shouldForceControlsDisplay) || ((this.layoutTraits & LayoutTraits.macOS) && this.isFullscreen));
     311
     312        if (!shouldControlsBeAvailable)
    312313            this._supportingObjects.forEach(supportingObject => supportingObject.disable());
    313314        else
    314315            this._supportingObjects.forEach(supportingObject => supportingObject.enable());
    315316
    316         this.controls.visible = !isFullscreen;
     317        this.controls.visible = shouldControlsBeAvailable;
    317318    }
    318319

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

    r218400 r228445  
    4545    }
    4646
     47    disable()
     48    {
     49        // We should not allow disabling Placard support when playing inline as it would prevent the
     50        // PiP placard from being shown if the controls are disabled.
     51        if (this.mediaController.isFullscreen)
     52            super.disable();
     53    }
     54
    4755    // Private
    4856
Note: See TracChangeset for help on using the changeset viewer.