[webkit-reviews] review granted: [Bug 193408] Add Reveal support in iOSMac : [Attachment 359212] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 193408: Add Reveal support in iOSMac
https://bugs.webkit.org/show_bug.cgi?id=193408

Attachment 359212: Patch

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




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

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

> Source/WebCore/ChangeLog:4
> +	   https://bugs.webkit.org/show_bug.cgi?id=193408

Where's the radar

> Source/WebCore/Configurations/WebCore.xcconfig:120
> +WK_UIKITMAC_LDFLAGS = $(WK_UIKITMAC_LDFLAGS_$(WK_PLATFORM_NAME));
> +WK_UIKITMAC_LDFLAGS_iosmac = -framework UIKitMacHelper;
> +

Remove this.

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:68
> +#include <pal/spi/ios/UIKitSPI.h>

I said something about this before.

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:180
> +    _highlighting = NO;

Technically not needed; ObjC zero fills objects.

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:232
> +    NSArray <NSValue *> *rects = [self highlightRectsForItem:item];

NSArray<

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:236
> +    UIView *textContentView = _view;

Maybe just use _view elsewhere instead of this local that is *more*  typing?

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:239
> +    for (NSUInteger i = 1; i < rects.count; i++)

This could use fast iteration! (for (NSValue *rect in rects)...)

> Source/WebCore/editing/cocoa/DictionaryLookup.mm:256
> +    

No newline :)