[webkit-reviews] review granted: [Bug 133186] -apple-system- styled element doesn't respond to system font size changes : [Attachment 231899] patch

[bugzilla-daemon at webkit.org]

Enrica Casucci <enrica at apple.com> has granted Martin Hock <mhock at apple.com>'s
request for review:
Bug 133186: -apple-system- styled element doesn't respond to system font size
changes
https://bugs.webkit.org/show_bug.cgi?id=133186

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

------- Additional Comments from Enrica Casucci <enrica at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231899&action=review


Looks good to me, provided you address the comments.

> Source/WebCore/ChangeLog:3
> +	   -apple-system- styled element doens't respond to system font size
changes.

typo: doesn't. We normally use the same title for all ChangeLogs in the same
patch.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:2272
> +		759CB837192DA9190012BC64 /* ControlStates.h in Headers */ =
{isa = PBXBuildFile; fileRef = 311C08BC18E35D6800B65615 /* ControlStates.h */;
settings = {ATTRIBUTES = (Private, ); }; };

What is this?

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:25764
> +				759CB837192DA9190012BC64 /* ControlStates.h in
Headers */,

What is this change?

> Source/WebCore/rendering/RenderThemeIOS.mm:315
>  CFStringRef RenderThemeIOS::contentSizeCategory()

No need to use CFStringRef here.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:165
> +    CFStringRef _userTextSize;

We normally use NSString.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1043
> +- (CFStringRef)_contentSizeCategory

Ditto. This way you don't need the cast on the return.