Bug 156358 – WebPlaybackControlsManager should support mediaSelectionOptions

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

[Beth Dakin]

WebPlaybackControlsManager should support mediaSelectionOptions

rdar://problem/25048743

[Beth Dakin]

Created attachment 275925 [details]
Patch

[WebKit Commit Bot]

Attachment 275925 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:101:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:102:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:103:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
ERROR: Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:104:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Beth Dakin]

Created attachment 275926 [details]
Patch

[Jer Noble]

Comment on attachment 275926 [details]
Patch

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

r=me, with possible nits.

> Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:269
> +        [webOption setLocalizedDisplayName:name];

I could be wrong, but I feel like the new preferred style guides for this kind of thing would read:

webOption.get().localizedDisplayName = name;

> Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:280
> +    [controlsManager setAudioMediaSelectionOptions:webOptions.get()];

...and:

controlsManager.get().audioMediaSelectionOptions = webOptions.get();

> Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm:282
> +        [controlsManager setCurrentAudioMediaSelectionOption:[webOptions objectAtIndex:static_cast<NSUInteger>(selectedIndex)]];

...and:

controlsManager.get().currentAudioMediaSelectionOption = [webOptions objectAtIndex:static_cast<NSUInteger>(selectedIndex)]];

etc.

But I may not be correct about that use of style.  Aside from than that, good patch!

[Beth Dakin]

Thanks Jer! http://trac.webkit.org/changeset/199184

My understanding has been that most people prefer not to use dot syntax with RetainPtrs, but it really is something we should address in the style guide. I chose not to address the comment for now.

http://trac.webkit.org/changeset/199184