<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Add long press selection test"
href="https://bugs.webkit.org/show_bug.cgi?id=162367#c2">Comment # 2</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Add long press selection test"
href="https://bugs.webkit.org/show_bug.cgi?id=162367">bug 162367</a>
from <span class="vcard"><a class="email" href="mailto:thorton@apple.com" title="Tim Horton <thorton@apple.com>"> <span class="fn">Tim Horton</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=289495&action=diff" name="attach_289495" title="Patch">attachment 289495</a> <a href="attachment.cgi?id=289495&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=289495&action=review">https://bugs.webkit.org/attachment.cgi?id=289495&action=review</a>
<span class="quote">> 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.</span >
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!
<span class="quote">> Tools/Scripts/webkitpy/common/config/contributors.json:3500
> + "Megan Gardner" : {</span >
This should happen in a different patch.
<span class="quote">> 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;</span >
Why?
<span class="quote">> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:548
> + [self longPressFinish:[[userInfo valueForKey:@"location"] CGPointValue] completionBlock:[userInfo objectForKey:@"block"]];</span >
I think we can userInfo[@"location"] and such.
<span class="quote">> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:549
> + [timer invalidate];</span >
No need to invalidate a non-repeating timer.
<span class="quote">> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:558
> + [self clearTap:CGPointMake(50, 50)];</span >
Why the random hardcoded 50, 50?
<span class="quote">> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:569
> + NSTimer *timer = [[NSTimer timerWithTimeInterval:longPressHoldDelay target:self selector:@selector(longPressTimerCall:) userInfo:info repeats:NO] retain];</span >
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).
<span class="quote">> LayoutTests/fast/events/touch/ios/long-press-to-select-text.html:33
> + var selection = document.getSelection().toString();</span >
Spaces, not tabs, please.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>