[webkit-reviews] review granted: [Bug 224683] [macOS] Add some support for webpage translation in WebKitLegacy : [Attachment 426299] Fix iOS 14/Big Sur builds (2)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 224683: [macOS] Add some support for webpage translation in WebKitLegacy
https://bugs.webkit.org/show_bug.cgi?id=224683

Attachment 426299: Fix iOS 14/Big Sur builds (2)

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




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 426299
  --> https://bugs.webkit.org/attachment.cgi?id=426299
Fix iOS 14/Big Sur builds (2)

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

> Source/WebKitLegacy/mac/WebView/WebView.mm:9652
> +    auto textToTranslate = adoptNS([[NSAttributedString alloc]
initWithString:text]);

We don’t need this local variable. Could just merge this line with the next.

> Source/WebKitLegacy/mac/WebView/WebView.mm:9655
> +	   [translationViewController setPreferredContentSize:NSMakeSize(400,
400)];

Hardcoded magic size without comment is not great.

> Source/WebKitLegacy/mac/WebView/WebView.mm:9670
> +    if (aim == highlight)

Seems wrong to ever compare two floating point values and expect them to be
exactly equal. Seems to me this should be some other kind of check, perhaps a
check that the absolute value is one pixel or more.