Bug 185260 – Some fields are not identified as [WKWebProcessPlugInNodeHandle isTextField]

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Richard Houle]

<rdar://problem/39290394> Some fields are not identified as [WKWebProcessPlugInNodeHandle isTextField]

INPUT element are not considered as text field when calling -[WKWebProcessPlugInNodeHandle isTextField] when they have attribute 'type="number"'.

[Richard Houle]

Created attachment 339429 [details]
Proposed fix. Instead of of using HTMLInputElement::isText, use HTMLInputElement::isTextElement.

[Richard Houle]

Daniel Bates pointed me that this bug was forseen in a FIXME comment in HTMLInputElement.h:

[[
    // FIXME: It's highly likely that any call site calling this function should instead
    // be using a different one. Many input elements behave like text fields, and in addition
    // any unknown input type is treated as text. Consider, for example, isTextField or
    // isTextField && !isPasswordField.
    WEBCORE_EXPORT bool isText() const;
]]

<https://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLInputElement.h?rev=231291#L112>

[Richard Houle]

Raising to P1, to match Safari's BRB priority.

[EWS Watchlist]

Comment on attachment 339429 [details]
Proposed fix. Instead of of using HTMLInputElement::isText, use HTMLInputElement::isTextElement.

Attachment 339429 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7553526

New failing tests:
fast/mediastream/delayed-permission-allowed.html

[EWS Watchlist]

Created attachment 339442 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 339429 [details]
Proposed fix. Instead of of using HTMLInputElement::isText, use HTMLInputElement::isTextElement.

Attachment 339429 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7559903

New failing tests:
http/tests/preload/onload_event.html

[EWS Watchlist]

Created attachment 339516 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Ryosuke Niwa]

Can we add an API test for this?

[Brent Fulgham]

These test failures do not seem to be related to this patch.

[Richard Houle]

Created attachment 339828 [details]
Proposed fix + API test

[Brent Fulgham]

Comment on attachment 339828 [details]
Proposed fix + API test

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

> Source/WebKit/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=185260

<rdar://problem/39290394>

> Source/WebKit/ChangeLog:6
> +        INPUT element are not considered as text field when

are not considered TO BE text fields when

> Tools/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=185260

<rdar://problem/39290394>

[Ryosuke Niwa]

Comment on attachment 339828 [details]
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?

[Ryosuke Niwa]

Comment on attachment 339828 [details]
Proposed fix + API test

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:631
> +    

Nit: Trailing whitespace.

[Richard Houle]

Created attachment 339907 [details]
V2 Proposed fix + API test

[Jessie Berlin]

Comment on attachment 339907 [details]
V2 Proposed fix + API test

Setting commit-queue+ since Richard isn't a WebKit committer (yet).

[WebKit Commit Bot]

Comment on attachment 339907 [details]
V2 Proposed fix + API test

Rejecting attachment 339907 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ntext = [[browserContextController mainFrame] jsContextForWorld:[WKWebProcessPlugInScriptWorld normalWorld]];
          ^
2 errors generated.

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/TestWebKitAPI.build/Release/WebProcessPlugIn.build/Objects-normal/x86_64/InjectedBundleNodeHandleIsTextField.o Tests/WebKitCocoa/InjectedBundleNodeHandleIsTextField.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.webkit.org/results/7617874

[Richard Houle]

Created attachment 340004 [details]
V3 Proposed fix + API test

[WebKit Commit Bot]

Comment on attachment 340004 [details]
V3 Proposed fix + API test

Clearing flags on attachment: 340004

Committed r231591: <https://trac.webkit.org/changeset/231591>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.