Bug 163238 – [Modern Media Controls] Buttons container

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

[Antoine Quint]

Media controls need a way to lay a group of buttons out together.

[Antoine Quint]

Created attachment 291160 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/28701864>

[Dean Jackson]

Comment on attachment 291160 [details]
Patch

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

> Source/WebCore/ChangeLog:30
> +        * Modules/modern-media-controls/controls/buttons-container.css: Copied from Source/WebCore/Modules/modern-media-controls/controls/airplay-button.js.
> +        (.buttons-container):
> +        * Modules/modern-media-controls/controls/buttons-container.js: Copied from Source/WebCore/Modules/modern-media-controls/controls/airplay-button.js.

Remove this Copied from junk.

> Source/WebCore/Modules/modern-media-controls/controls/buttons-container.js:64
> +            button.x = x;
> +            x += button.width + this.margin;

I don't think you should do work like this inside a call to filter, even though it is possible. I think you should treat the parameter as read only. A better way would be to call this.children.forEach after the filter.

[Antoine Quint]

(In reply to comment #3)
> Comment on attachment 291160 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291160&action=review
> 
> > Source/WebCore/Modules/modern-media-controls/controls/buttons-container.js:64
> > +            button.x = x;
> > +            x += button.width + this.margin;
> 
> I don't think you should do work like this inside a call to filter, even
> though it is possible. I think you should treat the parameter as read only.
> A better way would be to call this.children.forEach after the filter.

Cool, I'll have a children array that I'll populate during a forEach and then assign to the node's children property, so there's a single iteration.

[Antoine Quint]

Created attachment 291248 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 291248 [details]
Patch for landing

Clearing flags on attachment: 291248

Committed r207111: <http://trac.webkit.org/changeset/207111>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.