Bug 163539 – [Modern Media Controls] Basic MediaController

• Component: Media
• Status: RESOLVED FIXED

[Antoine Quint]

Now that we have all the controls figured out, we can create a very basic MediaController object that will be instantiated by the createControls() global function that is called from HTMLMediaElement::ensureMediaControlsInjectedScript() once the shadow root has been created and the appropriate scripts injected.

[Radar WebKit Bug Importer]

<rdar://problem/28797542>

[Antoine Quint]

Once https://bugs.webkit.org/show_bug.cgi?id=163500 is reviewed and lands, this patch will be updated to add the two new JS files to the WebCore Xcode project and the list of injected scripts.

[Antoine Quint]

Created attachment 291807 [details]
Patch

[Dean Jackson]

Comment on attachment 291807 [details]
Patch

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

> Source/WebCore/Modules/modern-media-controls/main.js:29
> +    if (!!host) {

What's the point of the !!? What value provided here would not be true as just (host)?

> Source/WebCore/Modules/modern-media-controls/media/media-controller.js:36
> +        this.layoutTraits = LayoutTraits.macOS;
> +
> +        this.controls = new MacOSInlineMediaControls

I think you should put a FIXME: here to make it clear it won't always be macOS.

And I just noticed the M in MacOSInlineMediaControls. Are you going to have IOSInlineMediaControls?

[Antoine Quint]

(In reply to comment #4)
> Comment on attachment 291807 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291807&action=review
> 
> > Source/WebCore/Modules/modern-media-controls/main.js:29
> > +    if (!!host) {
> 
> What's the point of the !!? What value provided here would not be true as
> just (host)?

Old habits, will fix.

> > Source/WebCore/Modules/modern-media-controls/media/media-controller.js:36
> > +        this.layoutTraits = LayoutTraits.macOS;
> > +
> > +        this.controls = new MacOSInlineMediaControls
> 
> I think you should put a FIXME: here to make it clear it won't always be
> macOS.
> 
> And I just noticed the M in MacOSInlineMediaControls. Are you going to have
> IOSInlineMediaControls?

Yes, that's right. Class names always start with capitals.

[Antoine Quint]

Created attachment 291873 [details]
Patch for landing

[Antoine Quint]

Created attachment 291874 [details]
Patch for landing

[Antoine Quint]

Created attachment 291876 [details]
Patch for landing

[Antoine Quint]

Created attachment 291877 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 291877 [details]
Patch for landing

Clearing flags on attachment: 291877

Committed r207436: <http://trac.webkit.org/changeset/207436>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.