[webkit-reviews] review granted: [Bug 202198] REGRESSION (r245672): <select> dropdown with text-rendering: optimizeLegibility freezes Safari : [Attachment 379982] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 202198: REGRESSION (r245672): <select> dropdown with text-rendering:
optimizeLegibility freezes Safari
https://bugs.webkit.org/show_bug.cgi?id=202198

Attachment 379982: Patch

https://bugs.webkit.org/attachment.cgi?id=379982&action=review




--- Comment #13 from Tim Horton <thorton at apple.com> ---
Comment on attachment 379982
  --> https://bugs.webkit.org/attachment.cgi?id=379982
Patch

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

> Source/WTF/wtf/Platform.h:1602
> +#define USE_CORETEXT_AUTO_OPTICAL_SIZING_WORKAROUND 1

Pretty sure this is a HAVE() not a USE() (which is about using features of the
platform, not internal workarounds)

HAVE_CORETEXT_WITH_BROKEN_AUTO_OPTICAL_SIZING or vice versa or something like
that

> Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:109
> +	   NSMutableDictionary *mutableDictionary = [(__bridge NSDictionary
*)data.fontInfo.fontAttributeDictionary.get() mutableCopy];

Why not a RetainPtr & adoptNS?

> Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:115
> +		   [mutableDictionary removeObjectForKey:(NSString
*)kCTFontOpticalSizeAttribute];
> +		   if (NSNumber *size = [mutableDictionary
objectForKey:(NSString *)kCTFontSizeAttribute])
> +		       [mutableDictionary setObject:size forKey:(NSString
*)kCTFontOpticalSizeAttribute];

Some might say if you're going to include one bridge you might as well bridge
them all. It's up to you since it really doesn't matter yet.