Summary: | WebPlaybackControlsManager should support mediaSelectionOptions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bdakin, commit-queue, eric.carlson, jer.noble | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Local Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Beth Dakin
2016-04-07 13:32:01 PDT
Created attachment 275925 [details]
Patch
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.
Created attachment 275926 [details]
Patch
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! 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 |