Bug 102861 – Implement ValidityState::badInput

• Component: Forms
• Status: RESOLVED FIXED

[Kent Tamura]

The WHATWG HTML specification has introduced ValidityState::badInput. We should implement it.

http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#dom-validitystate-badinput

[Kent Tamura]

Created attachment 175923 [details]
Patch

[Kent Tamura]

Created attachment 175924 [details]
Patch 2

Added a missing file

[Early Warning System Bot]

Comment on attachment 175924 [details]
Patch 2

Attachment 175924 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15000148

[Early Warning System Bot]

Comment on attachment 175924 [details]
Patch 2

Attachment 175924 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14989467

[Kent Tamura]

Created attachment 175931 [details]
Patch 3

Qt build fix

[EFL EWS Bot]

Comment on attachment 175931 [details]
Patch 3

Attachment 175931 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14982537

[Kent Tamura]

Created attachment 175933 [details]
Patch 4

EFL build fix

[Build Bot]

Comment on attachment 175933 [details]
Patch 4

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

New failing tests:
fast/forms/ValidityState-001.html

[Kentaro Hara]

Comment on attachment 175933 [details]
Patch 4

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

Looks good.

morrita-san: would you take another look?

> LayoutTests/fast/forms/number/number-validity-badinput.html:13
> +<p id="description"></p>
> +<div id="console"></div>

Nit: Remove this.

> LayoutTests/fast/forms/number/number-validity-badinput.html:29
> +shouldBeTrue('colorOf(number) != invalidStyleColor');

Nit: Use !==, or ShouldNotBe('colorOf(number)', 'invalidStyleColor')

> LayoutTests/fast/forms/number/number-validity-badinput.html:41
> +shouldBeTrue('colorOf(number) != invalidStyleColor');

Ditto.

> LayoutTests/fast/forms/resources/multiple-fields-validity-badinput.js:28
> +    if (type == 'date' || type== 'datetime' || type == 'datetime-local') {

Nit: type == 'datetime'. === would be better.

[Kent Tamura]

Created attachment 175940 [details]
Patch 5

Addressed reviewer comments. Fix a test failure.

[Kent Tamura]

Comment on attachment 175933 [details]
Patch 4

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

>> LayoutTests/fast/forms/number/number-validity-badinput.html:13
>> +<div id="console"></div>
> 
> Nit: Remove this.

Fixed.
FYI: I wanted to minimize the difference from number-unacceptable-style.html.

>> LayoutTests/fast/forms/number/number-validity-badinput.html:29
>> +shouldBeTrue('colorOf(number) != invalidStyleColor');
> 
> Nit: Use !==, or ShouldNotBe('colorOf(number)', 'invalidStyleColor')

Fixed.

>> LayoutTests/fast/forms/number/number-validity-badinput.html:41
>> +shouldBeTrue('colorOf(number) != invalidStyleColor');
> 
> Ditto.

Fixed.

>> LayoutTests/fast/forms/resources/multiple-fields-validity-badinput.js:28
>> +    if (type == 'date' || type== 'datetime' || type == 'datetime-local') {
> 
> Nit: type == 'datetime'. === would be better.

Fixed.

[Hajime Morrita]

Comment on attachment 175940 [details]
Patch 5

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

> Source/WebCore/ChangeLog:17
> +        interactive validation tells it.

Better to use consistent tense.

> Source/WebCore/ChangeLog:27
> +        will be added later.

The bug# might be worth being mentioned here.

> Source/WebCore/html/FormAssociatedElement.h:79
>      // stepMismatch, tooLong and valueMissing must call willValidate method.

Vaguely feel that it's time to fold these into one method which returns bit flags.

[Kent Tamura]

Committed r135836: <http://trac.webkit.org/changeset/135836>

[Kent Tamura]

Comment on attachment 175940 [details]
Patch 5

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

>> Source/WebCore/html/FormAssociatedElement.h:79
>>      // stepMismatch, tooLong and valueMissing must call willValidate method.
> 
> Vaguely feel that it's time to fold these into one method which returns bit flags.

MO, returning bit flags won't be nice.  When JS code gets input.validity.tooLong, we don't need to compute other validation states.

[Kent Tamura]

(In reply to comment #14)
> MO, returning bit flags won't be nice.  When JS code gets input.validity.tooLong, we don't need to compute other validation states.

MO -> IMO