[Webkit-unassigned] [Bug 162367] Add long press selection test

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=162367

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

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

> Tools/ChangeLog:31
> +        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
> +        (WTR::UIScriptController::longPressAtPoint):
> +        (WTR::UIScriptController::forcePressAtPoint):
> +        (WTR::UIScriptController::dragFromPointToPoint): Deleted.
> +        * Scripts/webkitpy/common/config/contributors.json:
> +        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
> +        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
> +        (WTR::UIScriptController::longPressAtPoint):
> +        (WTR::UIScriptController::forcePressAtPoint):
> +        (WTR::UIScriptController::dragFromPointToPoint): Deleted.
> +        * TestRunnerShared/UIScriptContext/UIScriptController.h:
> +        * WebKitTestRunner/ios/HIDEventGenerator.h:
> +        * WebKitTestRunner/ios/HIDEventGenerator.mm:
> +        (-[HIDEventGenerator _createIOHIDEventType:]):
> +        (-[HIDEventGenerator sendTaps:location:withNumberOfTouches:completionBlock:]):
> +        (-[HIDEventGenerator clearTap:]):
> +        (-[HIDEventGenerator longPressTimerCall:]):
> +        (-[HIDEventGenerator longPressFinish:completionBlock:]):
> +        (-[HIDEventGenerator longPress:completionBlock:]):
> +        (-[HIDEventGenerator forcePress:completionBlock:]):
> +        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
> +        (WTR::UIScriptController::longPressAtPoint):
> +        (WTR::UIScriptController::forcePressAtPoint):
> +        (WTR::UIScriptController::dragFromPointToPoint): Deleted.

This region is for explaining why/what you did in each of these places, making it easier to review and making it possible for me to ask less questions below!

> Tools/Scripts/webkitpy/common/config/contributors.json:3500
> +	  "Megan Gardner" : {

This should happen in a different patch.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:40
> -static const NSTimeInterval fingerLiftDelay = 5e7;
> -static const NSTimeInterval multiTapInterval = 15e7;
> +static const long fingerLiftDelay = 5e7;
> +static const long multiTapInterval = 15e7;

Why?

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:548
> +    [self longPressFinish:[[userInfo valueForKey:@"location"] CGPointValue] completionBlock:[userInfo objectForKey:@"block"]];

I think we can userInfo[@"location"] and such.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:549
> +    [timer invalidate];

No need to invalidate a non-repeating timer.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:558
> +    [self clearTap:CGPointMake(50, 50)];

Why the random hardcoded 50, 50?

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:569
> +    NSTimer *timer = [[NSTimer timerWithTimeInterval:longPressHoldDelay target:self selector:@selector(longPressTimerCall:) userInfo:info repeats:NO] retain];

Probably better to use scheduledTimerWith... and not have to explicitly add it, no? Also, I think the runloop will retain it in that case, so you don't have to (and don't have to release it when it fires).

> LayoutTests/fast/events/touch/ios/long-press-to-select-text.html:33
> +					var selection = document.getSelection().toString();

Spaces, not tabs, please.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160922/46be4c0f/attachment.html>