Bug 163678 – [Modern Media Controls] Media Controller: playback support

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Antoine Quint]

We need media controller support for the play/pause button and when the media is played and paused via the media API.

[Antoine Quint]

<rdar://problem/27989479>

[Antoine Quint]

Created attachment 292164 [details]
Patch

[Dean Jackson]

Comment on attachment 292164 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292164&action=review

> Source/WebCore/Modules/modern-media-controls/media/playback-support.js:54
> +class PlaybackSupport extends MediaControllerSupport
> +{
> +
> +    // Protected
> +
> +    get control()
> +    {
> +        return this.mediaController.controls.playPauseButton;
> +    }
> +
> +    get mediaEvents()
> +    {
> +        return ["play", "pause"];
> +    }
> +
> +    buttonWasClicked(control)
> +    {
> +        const media = this.mediaController.media;
> +        if (media.paused)
> +            media.play();
> +        else
> +            media.pause();
> +    }
> +
> +    syncControl()
> +    {
> +        this.control.playing = !this.mediaController.media.paused;
> +    }
> +

This is another example of a super simple Support class. I'm fairly worried about how confusing the end result of the approach will be. When it was in one huge file, you could at least search for the eventListener that was added to the play button.

[Antoine Quint]

(In reply to comment #3)
> Comment on attachment 292164 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=292164&action=review
> 
> > Source/WebCore/Modules/modern-media-controls/media/playback-support.js:54
> > +class PlaybackSupport extends MediaControllerSupport
> > +{
> > +
> > +    // Protected
> > +
> > +    get control()
> > +    {
> > +        return this.mediaController.controls.playPauseButton;
> > +    }
> > +
> > +    get mediaEvents()
> > +    {
> > +        return ["play", "pause"];
> > +    }
> > +
> > +    buttonWasClicked(control)
> > +    {
> > +        const media = this.mediaController.media;
> > +        if (media.paused)
> > +            media.play();
> > +        else
> > +            media.pause();
> > +    }
> > +
> > +    syncControl()
> > +    {
> > +        this.control.playing = !this.mediaController.media.paused;
> > +    }
> > +
> 
> This is another example of a super simple Support class. I'm fairly worried
> about how confusing the end result of the approach will be. When it was in
> one huge file, you could at least search for the eventListener that was
> added to the play button.

If you're searching for `playPauseButton`, you'll easy find this code just like you would have found it in a monolithic source file. But at least here the code related to the behaviour of the play/pause button is clearly contained.

[Antoine Quint]

Created attachment 292954 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 292954 [details]:

http/tests/security/svg-image-with-css-cross-domain.html bug 164029 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 292954 [details]
Patch for landing

Clearing flags on attachment: 292954

Committed r207909: <http://trac.webkit.org/changeset/207909>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.