Bug 177289 - Add long press and drag test
Summary: Add long press and drag test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Megan Gardner
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-20 19:52 PDT by Megan Gardner
Modified: 2018-05-04 14:22 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.06 KB, patch)
2017-09-20 19:57 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (7.98 KB, patch)
2017-10-02 19:15 PDT, Megan Gardner
wenson_hsieh: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Megan Gardner 2017-09-20 19:52:12 PDT
Add long press and drag test
Comment 1 Megan Gardner 2017-09-20 19:57:13 PDT
Created attachment 321402 [details]
Patch
Comment 2 Simon Fraser (smfr) 2017-09-20 20:42:17 PDT
Comment on attachment 321402 [details]
Patch

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

> LayoutTests/fast/events/touch/ios/long-press-then-drag-to-select-text.html:36
> +                                           output += 'FAIL: failed to select correct word as a result of a drag after long press. Incorrect selection: ' + document.getSelection().toString();

Odd spacing here. Tabs?

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:64
> +function longPressAtPointNoLift(X, Y)

The "NoLift" is awkward. Maybe pressAndHoldAtPoint?

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:70
> +               {

Canonical style would be to outdent opening and closing braces.

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:105
> +     uiController.sendEventStream(JSON.stringify(eventStream), function() {});
> +     uiController.uiScriptComplete();

These should be indented.

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:151
> +     uiController.sendEventStream(JSON.stringify(eventStream), function() {});
> +     uiController.uiScriptComplete();

Ditto.
Comment 3 Tim Horton 2017-09-20 21:05:13 PDT
Comment on attachment 321402 [details]
Patch

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

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:11
> +// should switch to this method in all tests, as it is more clear what is being done.

Comments should start with a capital and be a complete sentence :P (also, is this just a find-and-replace? Might be worth it to just do it? Or not, either way)

>> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:64
>> +function longPressAtPointNoLift(X, Y)
> 
> The "NoLift" is awkward. Maybe pressAndHoldAtPoint?

Agreed.
Comment 4 Megan Gardner 2017-09-21 11:48:47 PDT
https://trac.webkit.org/r222337
Comment 5 Radar WebKit Bug Importer 2017-09-27 12:19:54 PDT
<rdar://problem/34693068>
Comment 6 Matt Lewis 2017-09-27 17:07:17 PDT
Reverted r222337 for reason:

This test is failing on iOS.

Committed r222587: <http://trac.webkit.org/changeset/222587>
Comment 7 Megan Gardner 2017-10-02 19:15:23 PDT
Created attachment 322485 [details]
Patch
Comment 8 Wenson Hsieh 2017-10-03 14:53:42 PDT
Comment on attachment 322485 [details]
Patch

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

> LayoutTests/fast/events/touch/ios/long-press-then-drag-to-select-text.html:24
> +            var dragY = secondTargetRect.y+secondTargetRect.height / 2;

Spaces around binary operators! (i.e. the +s)

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:11
> +function longPressAtPointNoLift(X, Y)

I liked pressAndHoldAtPoint better than longPressAtPointNoLift :P

> LayoutTests/fast/events/touch/ios/resources/basic-gestures.js:118
> +               {

I think the formatting turned a bit Xcode-y here...can we make this consistent with the function above?
Comment 9 Megan Gardner 2018-05-04 14:22:47 PDT
Somehow this got left in a weird state.

https://trac.webkit.org/changeset/222813/webkit