[webkit-reviews] review granted: [Bug 224847] Long-pressing a data detectors link causes the link to be followed : [Attachment 426633] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 224847: Long-pressing a data detectors link causes the link to be followed
https://bugs.webkit.org/show_bug.cgi?id=224847

Attachment 426633: Patch

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




--- Comment #4 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 426633
  --> https://bugs.webkit.org/attachment.cgi?id=426633
Patch

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

r=mews

> Source/WebKit/Shared/NativeWebMouseEvent.h:73
> +    NativeWebMouseEvent(Type, Button, unsigned short buttons, const
WebCore::IntPoint& position, const WebCore::IntPoint& globalPosition, float
deltaX, float deltaY, float deltaZ, int clickCount, OptionSet<Modifier>,
WallTime timestamp, double force, bool gestureCancelled);

Should `gestureCancelled` be an enum class type instead of a bool?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:251
> +    RetainPtr<WKTestingTouch> touch = adoptNS([[WKTestingTouch alloc]
init]);

Nit - auto?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:255
> +    RetainPtr<WKTestingEvent> event = adoptNS([[WKTestingEvent alloc]
init]);

(Ditto)

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:268
> +    [webView _doAfterProcessingAllPendingMouseEvents:^{

Nit - this could be made slightly cleaner by using `[webView
waitForPendingMouseEvents];`

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:291
> +    RetainPtr<WKTestingTouch> touch = adoptNS([[WKTestingTouch alloc]
init]);

(Ditto)

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:295
> +    RetainPtr<WKTestingEvent> event = adoptNS([[WKTestingEvent alloc]
init]);

(Ditto)

> Tools/TestWebKitAPI/Tests/WebKitCocoa/iOSMouseSupport.mm:308
> +    [webView _doAfterProcessingAllPendingMouseEvents:^{

(Ditto)