[webkit-reviews] review denied: [Bug 177583] Add debug flag to WebKitTestRunner to show where touches are being generated : [Attachment 322055] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied  review:
Bug 177583: Add debug flag to WebKitTestRunner to show where touches are being
generated
https://bugs.webkit.org/show_bug.cgi?id=177583

Attachment 322055: Patch

https://bugs.webkit.org/attachment.cgi?id=322055&action=review




--- Comment #9 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 322055
  --> https://bugs.webkit.org/attachment.cgi?id=322055
Patch

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

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:204
> +	   newView.backgroundColor = [UIColor colorWithRed:1.0-i*.05 green:0.0
blue:1.0-i*.05 alpha:0.5];

Spaces around operators.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:207
> +	   [[[UIApplication sharedApplication] keyWindow] addSubview:newView];

Uh, this seems a bit random. I think we should explicitly pass a view to add
them to so we know the coordinate system is correct.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:299
> +- (void)updateDebugIndicatorForTouch:(UInt)index withPoint:(CGPoint)point
isTouching:(BOOL)isTouching

Point In what coordinate system? Method name should make that clear.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:338
> +	   if (self.shouldShowTouches)
> +	       [self
updateDebugIndicatorForTouch:[touchInfo[HIDEventTouchIDKey] intValue]
withPoint:CGPointMake([touchInfo[HIDEventXKey] floatValue],
[touchInfo[HIDEventYKey] floatValue]) isTouching:(BOOL)touch];

I don't think HIDEventGenerator should be in the business of doing UIView
stuff. I think this code should move somewhere else.