Bug 165119

Summary: Add coordinate space to event streams and streamline tests
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, megan_gardner, simon.fraser, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Megan Gardner 2016-11-28 15:41:11 PST
Add coordinate space to event streams and streamline tests
Comment 1 Megan Gardner 2016-11-28 15:48:32 PST
Created attachment 295543 [details]
Patch
Comment 2 Megan Gardner 2016-12-01 16:25:59 PST
Could someone take a quick look at this?
Comment 3 Tim Horton 2016-12-01 16:34:44 PST
Comment on attachment 295543 [details]
Patch

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

> Tools/ChangeLog:9
> +        Update example script to more accurately show all the avaialble options.

s/avaialble/available/

> Tools/WebKitTestRunner/ios/HIDEventGenerator.h:54
> +// Values for

?

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:63
> +NSString* const HIDEventCoordinateSpaceTypeContent = @"content";

technically there are a LOT of stars on the wrong side. but let's not fix in this patch.

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:211
> +            auto location = globalToContentCoordinates(TestController::singleton().mainWebView()->platformView(), (long)[touch[HIDEventXKey] doubleValue], (long)[touch[HIDEventYKey] doubleValue]);

Dot notation if you can.

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:212
> +            touch[HIDEventXKey] = [NSNumber numberWithFloat:location.x];

= @(location.x) maybe?

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:226
> +        if ([event[HIDEventCoordinateSpaceKey] isEqualToString:HIDEventCoordinateSpaceTypeContent]) {

early continue instead of a big indented block?
Comment 4 Simon Fraser (smfr) 2016-12-01 16:55:37 PST
Comment on attachment 295543 [details]
Patch

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

> Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:77
> +    //              "coordinateSpace" : "global",

or "screen"? Maybe this should takes scenes into account. What happens in multitasking mode?
Comment 5 Megan Gardner 2016-12-08 16:03:53 PST
Created attachment 296585 [details]
Patch
Comment 6 Megan Gardner 2016-12-09 14:10:47 PST
Created attachment 296697 [details]
Patch
Comment 7 Megan Gardner 2016-12-15 14:05:00 PST
Created attachment 297219 [details]
Patch
Comment 8 Simon Fraser (smfr) 2016-12-15 16:15:51 PST
Comment on attachment 297219 [details]
Patch

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

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:215
> +    if (event[HIDEventTouchesKey]) {
> +        for (NSMutableDictionary *touch in event[HIDEventTouchesKey]) {
> +            auto location = globalToContentCoordinates(TestController::singleton().mainWebView()->platformView(), (long)[touch[HIDEventXKey] doubleValue], (long)[touch[HIDEventYKey]doubleValue]);
> +            touch[HIDEventXKey] = @(location.x);
> +            touch[HIDEventYKey] = @(location.y);
> +        }
> +    }

If a test triggers a scroll in the middle, won't the subsequent content-relative locations be offset?
Comment 9 Tim Horton 2016-12-15 16:17:36 PST
Comment on attachment 297219 [details]
Patch

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

> Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:73
> +    //          //This is a basic force press

Comments have spaces after the //

> Tools/WebKitTestRunner/ios/HIDEventGenerator.h:54
> +// Values for

Values for what?

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:223
> +    auto eventInfo = dynamic_objc_cast<NSDictionary>([NSJSONSerialization JSONObjectWithData:[(NSString *)jsonString dataUsingEncoding:NSUTF8StringEncoding] options:NSJSONReadingMutableContainers |NSJSONReadingMutableLeaves error:nil]);

Space after the |

> LayoutTests/fast/events/touch/ios/long-press-then-drag-down-to-change-selected-text.html:82
> +                    testRunner.runUIScript(getDragScript(72,120,72,140), function(result) {

Spaces after commas
Comment 10 Megan Gardner 2016-12-15 16:33:25 PST
Created attachment 297251 [details]
Patch
Comment 11 WebKit Commit Bot 2016-12-15 17:15:16 PST
Comment on attachment 297251 [details]
Patch

Clearing flags on attachment: 297251

Committed r209892: <http://trac.webkit.org/changeset/209892>
Comment 12 WebKit Commit Bot 2016-12-15 17:15:21 PST
All reviewed patches have been landed.  Closing bug.