Bug 163500 - [Modern Media Controls] Use modern-media-controls module sources for media controls stylesheet and script injection
Summary: [Modern Media Controls] Use modern-media-controls module sources for media co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Safari 10
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks: 163501 163502 163539
  Show dependency treegraph
 
Reported: 2016-10-16 06:39 PDT by Antoine Quint
Modified: 2016-10-17 11:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (37.86 KB, patch)
2016-10-16 06:51 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (35.12 KB, patch)
2016-10-17 10:58 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2016-10-16 06:39:42 PDT
We need to update RenderTheme to use the modern-media-controls module when the runtime flag is on.
Comment 1 Radar WebKit Bug Importer 2016-10-16 06:40:04 PDT
<rdar://problem/28792010>
Comment 2 Antoine Quint 2016-10-16 06:51:30 PDT
Created attachment 291745 [details]
Patch
Comment 3 Antoine Quint 2016-10-17 10:58:48 PDT
Created attachment 291834 [details]
Patch for landing
Comment 4 Dean Jackson 2016-10-17 11:28:28 PDT
Comment on attachment 291834 [details]
Patch for landing

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

> Source/WebCore/rendering/RenderThemeMac.mm:241
> +            NSArray* paths = @[@"airplay-button", @"button", @"buttons-container", @"icon-button", @"macos-inline-media-controls", @"media-controls", @"placard", @"scrubber", @"slider", @"start-button", @"time-label", @"volume-slider"];
> +            for (NSString* path in paths)

These should be NSArray *paths and NSString *path. See https://webkit.org/code-style-guidelines/#pointers-and-references (although there is little consistency throughout WebKit here)

> Source/WebCore/rendering/RenderThemeMac.mm:261
> +            NSArray* controlsPaths = @[@"scheduler", @"layout-node", @"layout-item", @"icon-service", @"time-control", @"time-label", @"slider", @"volume-slider", @"scrubber", @"button", @"start-button", @"icon-button", @"play-pause-button", @"skip-back-button", @"mute-button", @"airplay-button", @"pip-button", @"tracks-button", @"fullscreen-button", @"aspect-ratio-button", @"rewind-button", @"forward-button", @"media-controls", @"macos-media-controls", @"macos-inline-media-controls", @"buttons-container", @"placard", @"airplay-placard", @"pip-placard"];
> +            for (NSString* path in controlsPaths)

Same here.
Comment 5 WebKit Commit Bot 2016-10-17 11:29:31 PDT
Comment on attachment 291834 [details]
Patch for landing

Clearing flags on attachment: 291834

Committed r207418: <http://trac.webkit.org/changeset/207418>
Comment 6 WebKit Commit Bot 2016-10-17 11:29:36 PDT
All reviewed patches have been landed.  Closing bug.