Bug 163479

Summary: Add support for reportValidity() on form and form control elements
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, dbates, lforschler, manian, rniwa, sam, tkent, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://html.spec.whatwg.org/#dom-cva-reportvalidity
Bug Depends on: 28649    
Bug Blocks: 164382, 163550    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch none

Description Chris Dumez 2016-10-14 19:53:06 PDT
Add support for reportValidity() on form and form control elements:
- https://html.spec.whatwg.org/#dom-cva-reportvalidity

Firefox and Chrome already support this.
Comment 1 Chris Dumez 2016-10-14 19:53:23 PDT
demo: https://googlechrome.github.io/samples/report-validity/
Comment 2 Chris Dumez 2016-10-14 20:52:03 PDT
Created attachment 291704 [details]
Patch
Comment 3 Sam Weinig 2016-10-14 21:09:27 PDT
Comment on attachment 291704 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.cpp:512
> +        String message("An invalid form control with name='%name' is not focusable.");
> +        message.replace("%name", name());

This could be more efficient if you used makeString("An invalid form control with name=', name(), "' is not focusable.")
Comment 4 Build Bot 2016-10-14 22:06:19 PDT
Comment on attachment 291704 [details]
Patch

Attachment 291704 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2288377

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-reportValidity.html
Comment 5 Build Bot 2016-10-14 22:06:22 PDT
Created attachment 291706 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 6 Chris Dumez 2016-10-14 22:31:56 PDT
Created attachment 291708 [details]
Patch
Comment 7 Build Bot 2016-10-14 23:45:40 PDT
Comment on attachment 291708 [details]
Patch

Attachment 291708 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2288770

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-reportValidity.html
Comment 8 Build Bot 2016-10-14 23:45:44 PDT
Created attachment 291710 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 9 Chris Dumez 2016-10-15 08:43:52 PDT
Created attachment 291718 [details]
Patch
Comment 10 WebKit Commit Bot 2016-10-15 14:34:39 PDT
Comment on attachment 291718 [details]
Patch

Clearing flags on attachment: 291718

Committed r207380: <http://trac.webkit.org/changeset/207380>
Comment 11 WebKit Commit Bot 2016-10-15 14:34:44 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Kent Tamura 2016-10-17 01:33:58 PDT
Implementing reportValidity() without enabling interactive validation is very inconsistent.

Does this reportValidity() implementation use the old ShadowDOM-based bubble?
Comment 13 Chris Dumez 2016-10-17 09:06:42 PDT
(In reply to comment #12)
> Implementing reportValidity() without enabling interactive validation is
> very inconsistent.

Oh, I did not realize that interactive validation was disabled. Let me look into it. I agree that both should be enabled for consistency.

> 
> Does this reportValidity() implementation use the old ShadowDOM-based bubble?

I have not worked on this part, I basically reused code from interactive form validation to make reportValidity() work. I tested that bubbles showed as expected.
I have just looked into it and it does appear it currently uses an old ShadowDOM based implementation (that you wrote a long time ago). I need to talk to people about this as I assume this may need improvements.
Comment 14 Radar WebKit Bug Importer 2016-10-17 11:03:03 PDT
<rdar://problem/28803331>
Comment 15 Kent Tamura 2016-10-17 19:14:10 PDT
(In reply to comment #13)
> I have just looked into it and it does appear it currently uses an old
> ShadowDOM based implementation (that you wrote a long time ago). I need to
> talk to people about this as I assume this may need improvements.

As I wrote in https://bugs.webkit.org/show_bug.cgi?id=95527#c0, The ShadowDOM implementation had multiple problems.  I strongly recommend to implement ValidationMessageClient with native widgets.
Comment 16 Chris Dumez 2016-10-17 20:22:50 PDT
(In reply to comment #15)
> (In reply to comment #13)
> > I have just looked into it and it does appear it currently uses an old
> > ShadowDOM based implementation (that you wrote a long time ago). I need to
> > talk to people about this as I assume this may need improvements.
> 
> As I wrote in https://bugs.webkit.org/show_bug.cgi?id=95527#c0, The
> ShadowDOM implementation had multiple problems.  I strongly recommend to
> implement ValidationMessageClient with native widgets.

Yes, I understand, thanks. I'm currently discussing this with others.