Bug 163678

<rdar://problem/27989479>

Created attachment 292164 [details] Patch

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.

(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.

Created attachment 292954 [details] Patch for landing

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: sabouhallawa@apple.com) The commit-queue is continuing to process your patch.

Comment on attachment 292954 [details] Patch for landing Clearing flags on attachment: 292954 Committed r207909: <http://trac.webkit.org/changeset/207909>

All reviewed patches have been landed. Closing bug.