Bug 85704 - Bad checkValidity result on recently "enabled" form fields
Summary: Bad checkValidity result on recently "enabled" form fields
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Keishi Hattori
URL: http://jsfiddle.net/9bsZ9/
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-05 03:50 PDT by alexander farkas
Modified: 2012-05-25 02:31 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.56 KB, patch)
2012-05-24 06:54 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (24.78 KB, patch)
2012-05-24 22:15 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (24.41 KB, patch)
2012-05-24 23:02 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (4.59 KB, patch)
2012-05-25 01:16 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description alexander farkas 2012-05-05 03:50:14 PDT
The native checkValidity() method and jQuery's is method for the selectors ':valid'/':invalid' are returning wrong results, if you set an invalid value to a disabled form field and then enabling it.

I created a testcase here: http://jsfiddle.net/9bsZ9/

1. Create a disabled and required form field
2. Set this field to an invalid value through script
3. Set disabled to false

-> checkValidity should return false, but returns true. I'm not sure, why matchesSelector and jQuery.is are returning different values, but should also be looked into.
Comment 1 Kent Tamura 2012-05-06 18:33:09 PDT
Confirmed.
Comment 2 Kent Tamura 2012-05-24 02:57:58 PDT
Hattori-san, can you handle this?
Comment 3 Keishi Hattori 2012-05-24 06:54:48 PDT
Created attachment 143821 [details]
Patch
Comment 4 Kent Tamura 2012-05-24 08:36:21 PDT
Comment on attachment 143821 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        willValidate which is const, so I added the member

it's ok to remove const from willValidate().  Then, we can assume m_willValidateInitialized as willValidate-and-validity-are-initialized.
Comment 5 Keishi Hattori 2012-05-24 22:15:47 PDT
Created attachment 143975 [details]
Patch
Comment 6 WebKit Review Bot 2012-05-24 22:19:03 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 7 Kent Tamura 2012-05-24 22:22:22 PDT
Comment on attachment 143975 [details]
Patch

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

> Source/WebCore/html/HTMLFormControlElement.h:98
>  
> -    virtual bool willValidate() const;
> +    virtual bool willValidate();
>      void updateVisibleValidationMessage();

should add OVERRIDE.

> Source/WebCore/html/HTMLFormControlElement.h:162
>  
> -    enum DataListAncestorState { Unknown, InsideDataList, NotInsideDataList };
> +    enum DataListAncestorState { Unknown, InsideDataList, NotInsideDataList, DontCare };
>      mutable enum DataListAncestorState m_dataListAncestorState;

This change is unrelated to this bug.

> Source/WebCore/html/HTMLKeygenElement.h:37
> +    virtual bool willValidate() { return false; }

OVERRIDE

> Source/WebCore/html/HTMLOutputElement.h:44
> +    virtual bool willValidate() { return false; }

ditto.
Comment 8 Keishi Hattori 2012-05-24 23:02:01 PDT
Created attachment 143982 [details]
Patch
Comment 9 Kent Tamura 2012-05-24 23:15:27 PDT
Comment on attachment 143982 [details]
Patch

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

> Source/WebKit/chromium/public/WebInputElement.h:93
>          WEBKIT_EXPORT int selectionEnd() const;
> -        WEBKIT_EXPORT bool isValidValue(const WebString&) const;
> +        WEBKIT_EXPORT bool isValidValue(const WebString&);
>          WEBKIT_EXPORT bool isChecked() const;

It's possible that this change breaks Chromium build.  Please watch the canary bots after landing this.
Comment 10 Keishi Hattori 2012-05-25 01:16:55 PDT
Created attachment 143998 [details]
Patch
Comment 11 WebKit Review Bot 2012-05-25 02:31:06 PDT
Comment on attachment 143998 [details]
Patch

Clearing flags on attachment: 143998

Committed r118501: <http://trac.webkit.org/changeset/118501>
Comment 12 WebKit Review Bot 2012-05-25 02:31:12 PDT
All reviewed patches have been landed.  Closing bug.