Bug 165119 – Add coordinate space to event streams and streamline tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Add coordinate space to event streams and streamline tests

[Megan Gardner]

Created attachment 295543 [details]
Patch

[Megan Gardner]

Could someone take a quick look at this?

[Tim Horton]

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?

[Simon Fraser (smfr)]

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?

[Megan Gardner]

Created attachment 296585 [details]
Patch

[Megan Gardner]

Created attachment 296697 [details]
Patch

[Megan Gardner]

Created attachment 297219 [details]
Patch

[Simon Fraser (smfr)]

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?

[Tim Horton]

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

[Megan Gardner]

Created attachment 297251 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 297251 [details]
Patch

Clearing flags on attachment: 297251

Committed r209892: <http://trac.webkit.org/changeset/209892>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.