Bug 27452

Summary: Support for the checkValidity() method and the invalid event
Product: WebKit Reporter: Michelangelo De Simone <michelangelo>
Component: FormsAssignee: Michelangelo De Simone <michelangelo>
Status: RESOLVED FIXED    
Severity: Enhancement CC: adele, darin, marcoos+bwo, pkasting, tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#dom-form-checkvalidity
Bug Depends on:    
Bug Blocks: 19264, 28649    
Attachments:
Description Flags
Untested, undocumented and unworthy patch
none
Patch v1
none
Patch v1 with updated layout tests
none
Patch v1 with working test case
none
Patch v1a
none
Patch v1b adele: review+

Description Michelangelo De Simone 2009-07-20 11:59:10 PDT
As per summary the implementation of checkValidity method along with its related simple event "invalid" is required as soon as static validation routine are landed.
Comment 1 Michelangelo De Simone 2009-08-18 18:43:15 PDT
Created attachment 35093 [details]
Untested, undocumented and unworthy patch
Comment 2 Michelangelo De Simone 2009-08-18 18:49:12 PDT
Peter and I discussed the Form::checkValidity() method on #WebKit, it has an optional parameter (unhandledInvalidControls) which can be used for interactive validation tkent is going to work (or is already working) on; see 4.10.15.2.

I'd like some feedback and, yes, the comment inside HTMLFormElement.idl is a mistake.:)
Comment 3 Peter Kasting 2009-08-18 18:55:29 PDT
Comment on attachment 35093 [details]
Untested, undocumented and unworthy patch

> diff --git a/WebCore/dom/Document.idl b/WebCore/dom/Document.idl
> index 44966cc..116be9f 100644
> --- a/WebCore/dom/Document.idl
> +++ b/WebCore/dom/Document.idl
> @@ -277,6 +277,7 @@ module core {
>          attribute [DontEnum] EventListener onscroll;
>          attribute [DontEnum] EventListener onselect;
>          attribute [DontEnum] EventListener onsubmit;
> +        attribute [DontEnum] EventListener oninvalid;

Alphabetical order?

> diff --git a/WebCore/dom/Element.idl b/WebCore/dom/Element.idl
> index 16aac84..44a73ae 100644
> --- a/WebCore/dom/Element.idl
> +++ b/WebCore/dom/Element.idl
> @@ -162,6 +162,7 @@ module core {
>          attribute [DontEnum] EventListener onscroll;
>          attribute [DontEnum] EventListener onselect;
>          attribute [DontEnum] EventListener onsubmit;
> +        attribute [DontEnum] EventListener oninvalid;

Here too

> diff --git a/WebCore/dom/Node.h b/WebCore/dom/Node.h
> index a557f94..2276c0f 100644
> --- a/WebCore/dom/Node.h
> +++ b/WebCore/dom/Node.h
> @@ -613,6 +613,8 @@ public:
>      void setOnselect(PassRefPtr<EventListener>);
>      EventListener* onsubmit() const;
>      void setOnsubmit(PassRefPtr<EventListener>);
> +    EventListener* oninvalid() const;
> +    void setOninvalid(PassRefPtr<EventListener>);

Here too (maybe should put the existing cases in order...?)

> diff --git a/WebCore/page/DOMWindow.idl b/WebCore/page/DOMWindow.idl
> index 7c61523..e9c6ac5 100644
> --- a/WebCore/page/DOMWindow.idl
> +++ b/WebCore/page/DOMWindow.idl
> @@ -250,6 +250,7 @@ module window {
>          attribute EventListener onunload;
>          attribute EventListener onvolumechange;
>          attribute EventListener onwaiting;
> +        attribute EventListener oninvalid;

Here too
Comment 4 Kent Tamura 2009-08-19 03:12:47 PDT
(In reply to comment #2)
> Peter and I discussed the Form::checkValidity() method on #WebKit, it has an
> optional parameter (unhandledInvalidControls) which can be used for interactive
> validation tkent is going to work (or is already working) on; see 4.10.15.2.

It seems good.  Thanks!
Comment 5 Michelangelo De Simone 2009-08-19 13:10:51 PDT
CC'ed Darin to have his comments too.
Comment 6 Michelangelo De Simone 2009-08-19 15:42:21 PDT
Created attachment 35152 [details]
Patch v1
Comment 7 Michelangelo De Simone 2009-08-19 16:09:08 PDT
Comment on attachment 35152 [details]
Patch v1

Regression on (the usual) domListEnumeration: updating...
Comment 8 Michelangelo De Simone 2009-08-19 16:17:24 PDT
Created attachment 35157 [details]
Patch v1 with updated layout tests
Comment 9 Michelangelo De Simone 2009-08-21 13:52:53 PDT
There's a mistake in one of the attached tests.
Comment 10 Michelangelo De Simone 2009-08-21 14:57:10 PDT
Created attachment 38398 [details]
Patch v1 with working test case
Comment 11 Michelangelo De Simone 2009-08-21 15:00:09 PDT
Created attachment 38400 [details]
Patch v1a

Bugzilla-tool didn't do the trick...:)
Comment 12 Michelangelo De Simone 2009-08-21 15:35:13 PDT
Adele gave her feedbacks on irc, the patch is being modified with the elimination of the stuff related to unhandledInvalidControls; instead of it a "TODO" comment is added to remind about it.
Comment 13 Michelangelo De Simone 2009-08-21 16:16:35 PDT
Created attachment 38408 [details]
Patch v1b

This patch complies with Adele's review comments.
Comment 14 Adele Peterson 2009-08-21 16:22:18 PDT
Comment on attachment 38408 [details]
Patch v1b

You can return false early if !control->checkValidity().  Everything else looks good. r=me

> +bool HTMLFormElement::checkValidity()
> +{
> +    // TODO: Check for unhandled invalid controls, see #27452 for tips.
> +
> +    bool hasOnlyValidControls = true;
> +    for (unsigned i = 0; i < formElements.size(); ++i) {
> +        HTMLFormControlElement* control = formElements[i];
> +        if (!control->checkValidity())
> +            hasOnlyValidControls = false;
> +    }
> +
> +    return hasOnlyValidControls;
> +}
> +
Comment 15 Michelangelo De Simone 2009-08-21 16:23:54 PDT
(In reply to comment #14)

> (From update of attachment 38408 [details])
> You can return false early if !control->checkValidity().  Everything else looks
> good. r=me

I can't. Form::checkValidity() is supposed to fire an invalid event on every single invalid controls it has.
Comment 16 Adele Peterson 2009-08-21 16:26:00 PDT
nevermind my comment - I forgot about where the events were dispatched
Comment 17 Peter Kasting 2009-08-21 16:56:05 PDT
Fixed in r47649.