[webkit-reviews] review granted: [Bug 185260] Some fields are not identified as [WKWebProcessPlugInNodeHandle isTextField] : [Attachment 339828] Proposed fix + API test

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Jessie Berlin
<jberlin at webkit.org>'s request for review:
Bug 185260: Some fields are not identified as [WKWebProcessPlugInNodeHandle
isTextField]
https://bugs.webkit.org/show_bug.cgi?id=185260

Attachment 339828: Proposed fix + API test

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




--- Comment #13 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 339828
  --> https://bugs.webkit.org/attachment.cgi?id=339828
Proposed fix + API test

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/IsTextField.mm:57
> +    

Nit: trailing whitespace.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/IsTextField.mm:66
> +    

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/IsTextField.mm:68
> +    BOOL results = YES;
> +    results &= [self isTextFieldForHTMLInputType:@"date" document:document
jsContext:jsContext];

This would make it impossible to tell which test case had failed when they do.
Please generate an assert for each test case instead.
Alternatively, you can use WKBundlePostSynchronousMessage.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:628
> +TEST(WebKit, IsTextField)

This is rather a generic name. How about InjectedBundleNodeHandleIsTextField?