[webkit-reviews] review granted: [Bug 177551] Build fix for High Sierra 32 bit Mac : [Attachment 321976] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 177551: Build fix for High Sierra 32 bit Mac
https://bugs.webkit.org/show_bug.cgi?id=177551

Attachment 321976: Patch

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




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 321976
  --> https://bugs.webkit.org/attachment.cgi?id=321976
Patch

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

There are lots of other opportunities for modernizing the code here. That's
probably not worth it, as we may well survive another 15 years without updating
it.

> Source/WebKitLegacy/mac/Carbon/CarbonWindowAdapter.mm:250
> +    

There are quite a few lines with trailing spaces, would be nice to get rid of
them.

> Source/WebKitLegacy/mac/Carbon/CarbonWindowAdapter.mm:501
> +    if (osStatus==noErr && defaultButton)

Missing spaces around ==

> Source/WebKitLegacy/mac/Carbon/CarbonWindowAdapter.mm:656
> +    view = [HIViewAdapter getHIViewForNSView:aView];\

Spurious backslash here.

Please take a closer look at the other style checker warnings - it's usually
not wrong about braces in multiline line control clauses.

> Source/WebKitLegacy/mac/Carbon/CarbonWindowAdapter.mm:669
> +    HIViewRef view = NULL;

We don't use NULL, should be nullptr or nil for Objective-C.

> Source/WebKitLegacy/mac/Carbon/CarbonWindowAdapter.mm:762
> +    if (eventType==NSAppKitDefined)
> +{

There should be indentation here. Missing spaces around ==.