Changeset 228947 in webkit


Ignore:
Timestamp:
Feb 23, 2018 5:53:13 AM (6 years ago)
Author:
graouts@webkit.org
Message:

REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
​https://bugs.webkit.org/show_bug.cgi?id=183074
<rdar://problem/37747028>

Reviewed by Eric Carlson.

Source/WebCore:

Test: media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html

In the fix for webkit.org/b/182668, we made it so that when the "controls" attribute is absent from a media
element we stop listening to the bulk of media events and prevent controls from updating any DOM properties
so as to minimize the amount of CPU usage by the Web process.

An unfortunate side effect was that, if the media controls were disabled at the time the video starts playing,
the StartSupport class would thus not catch the "play" event and would not be able to set the "hasPlayed"
property to "true" on the MediaController, which would then prevent the _shouldShowStartButton() from returning
"false". As a result, if the "controls" attribute was turned back on after the media started playing, they
would default to showing the start button, which would be then in the play state, ie. showing the pause icon.

We now set the "hasPlayed" property in the "play" event handler on MediaController, which is always registered
regardless of the "controls" attribute setting. We also ensure we invalidate the "showStartButton" property on
the media controls when StartSupport is enabled, which is the case when the "controls" attribute is toggled back
to "true" from a previous "false" value.

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

(MediaController.prototype.handleEvent):

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

(StartSupport):
(StartSupport.prototype.enable):
(StartSupport.prototype.handleEvent):
(StartSupport.prototype._updateShowsStartButton):

LayoutTests:

Add a new test that set controls on the video, then immediately removes them, plays the video and turns the controls
back on as soon as the video starts to check that the "showsStartButton" property is false on the media controls.
Prior to this patch this test would fail.

  • media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt: Added.
  • media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html: Added.
  • platform/ios/TestExpectations:
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r228935 r228947  
     12018-02-22  Antoine Quint  <graouts@apple.com>
     2
     3        REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
     4        https://bugs.webkit.org/show_bug.cgi?id=183074
     5        <rdar://problem/37747028>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Add a new test that set controls on the video, then immediately removes them, plays the video and turns the controls
     10        back on as soon as the video starts to check that the "showsStartButton" property is false on the media controls.
     11        Prior to this patch this test would fail.
     12
     13        * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt: Added.
     14        * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html: Added.
     15        * platform/ios/TestExpectations:
     16
    1172018-02-22  Youenn Fablet  <youenn@apple.com>
    218

  • trunk/LayoutTests/platform/ios/TestExpectations

    r228883 r228947  
    32013201media/modern-media-controls/seek-forward-support [ Skip ]
    32023202media/modern-media-controls/start-support/start-support-click-to-start.html [ Skip ]
     3203media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html [ Skip ]
    32033204media/modern-media-controls/start-support/start-support-lowPowerMode.html [ Skip ]
    32043205media/modern-media-controls/volume-down-support [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r228946 r228947  
     12018-02-22  Antoine Quint  <graouts@apple.com>
     2
     3        REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
     4        https://bugs.webkit.org/show_bug.cgi?id=183074
     5        <rdar://problem/37747028>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Test: media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html
     10
     11        In the fix for webkit.org/b/182668, we made it so that when the "controls" attribute is absent from a media
     12        element we stop listening to the bulk of media events and prevent controls from updating any DOM properties
     13        so as to minimize the amount of CPU usage by the Web process.
     14
     15        An unfortunate side effect was that, if the media controls were disabled at the time the video starts playing,
     16        the StartSupport class would thus not catch the "play" event and would not be able to set the "hasPlayed"
     17        property to "true" on the MediaController, which would then prevent the _shouldShowStartButton() from returning
     18        "false". As a result, if the "controls" attribute was turned back on after the media started playing, they
     19        would default to showing the start button, which would be then in the play state, ie. showing the pause icon.
     20
     21        We now set the "hasPlayed" property in the "play" event handler on MediaController, which is always registered
     22        regardless of the "controls" attribute setting. We also ensure we invalidate the "showStartButton" property on
     23        the media controls when StartSupport is enabled, which is the case when the "controls" attribute is toggled back
     24        to "true" from a previous "false" value.
     25
     26        * Modules/modern-media-controls/media/media-controller.js:
     27        (MediaController.prototype.handleEvent):
     28        * Modules/modern-media-controls/media/start-support.js:
     29        (StartSupport):
     30        (StartSupport.prototype.enable):
     31        (StartSupport.prototype.handleEvent):
     32        (StartSupport.prototype._updateShowsStartButton):
     33
    1342018-02-23  Carlos Garcia Campos  <cgarcia@igalia.com>
    235

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

    r228718 r228947  
    170170            scheduler.flushScheduledLayoutCallbacks();
    171171        } else if (event.currentTarget === this.media) {
     172            if (event.type === "play")
     173                this.hasPlayed = true;
    172174            this._updateControlsIfNeeded();
    173175            this._updateControlsAvailability();

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

    r219621 r228947  
    3131        super(mediaController);
    3232
    33         this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
     33        this._updateShowsStartButton();
    3434    }
    3535
    … …  
    4141    }
    4242
     43    enable()
     44    {
     45        super.enable();
     46
     47        this._updateShowsStartButton();
     48    }
     49
    4350    buttonWasPressed(control)
    4451    {
    … …  
    4855    handleEvent(event)
    4956    {
    50         if (event.type === "play")
    51             this.mediaController.hasPlayed = true;
     57        super.handleEvent(event);
    5258
    53         this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
    54 
    55         super.handleEvent(event);
     59        this._updateShowsStartButton();
    5660    }
    5761
    5862    // Private
     63
     64    _updateShowsStartButton()
     65    {
     66        this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
     67    }
    5968
    6069    _shouldShowStartButton()
Note: See TracChangeset for help on using the changeset viewer.