[webkit-reviews] review granted: [Bug 118076] [Windows] Enable CaptionUserPreferencesMediaAF on Windows : [Attachment 205524] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 118076: [Windows] Enable CaptionUserPreferencesMediaAF on Windows
https://bugs.webkit.org/show_bug.cgi?id=118076

Attachment 205524: Patch
https://bugs.webkit.org/attachment.cgi?id=205524&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=205524&action=review


> Source/WTF/wtf/Platform.h:957
> +#if (PLATFORM(MAC) || (OS(WINDOWS) && USE(CG))) && !PLATFORM(IOS) &&
__MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
>  #define WTF_USE_AVFOUNDATION 1
>  #endif

Did you mean to include this in this patch?

> Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:139
> +SOFT_LINK_DLL_IMPORT(CoreText, CTFontDescriptorCopyAttribute,  CFTypeRef,
__cdecl, (CTFontDescriptorRef descriptor, CFStringRef attribute), (descriptor,
attribute));
> +#define CTFontDescriptorCopyAttribute softLink_CTFontDescriptorCopyAttribute

> +
> +SOFT_LINK_VARIABLE_DLL_IMPORT(CoreText, kCTFontNameAttribute, CFStringRef)
> +#define kCTFontNameAttribute get_kCTFontNameAttribute()
> +

Doesn't this need to be inside of "PLATFORM(WIN)"?

> Source/WebCore/platform/LocalizedStrings.cpp:1055
> +#if USE(CF)

Why "USE(CF)" instead of Mac and Windows explicitly?

> Source/WebCore/platform/LocalizedStrings.h:246
> -#if PLATFORM(MAC)
> +#if USE(CF)

Ditto.