Summary: | [Modern Media Controls] Buttons container | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
Component: | Media | Assignee: | Antoine Quint <graouts> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, graouts, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Safari 10 | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Antoine Quint
2016-10-10 14:02:46 PDT
Created attachment 291160 [details]
Patch
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. (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. Created attachment 291248 [details]
Patch for landing
Comment on attachment 291248 [details] Patch for landing Clearing flags on attachment: 291248 Committed r207111: <http://trac.webkit.org/changeset/207111> All reviewed patches have been landed. Closing bug. |