Bug 50663 – Improve validation API support of <object> and <keygen>

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kent Tamura]

http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-object-element

We should add
 - validity
 - validationMessage
 - checkValidity()
 - setCustomValidity()

[Kenichi Ishibashi]

Created attachment 75997 [details]
Patch V0

[Kenichi Ishibashi]

This patch includes validation API tests for <output> and <keygen> since it seems that there is no test for them. For <keygen>, the willValidate property returns true for now, but I think it should be false because it is barred from constraint validation, as the HTML5 spec specified (http://dev.w3.org/html5/spec/the-button-element.html#the-keygen-element). I'll file a bug if my understand correctly.

(In reply to comment #1)
> Created an attachment (id=75997) [details]
> Patch V0

[Kent Tamura]

Comment on attachment 75997 [details]
Patch V0

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

> LayoutTests/fast/forms/ValidityState-001-expected.txt:7
> +SUCCESS
> +SUCCESS
> +SUCCESS
>  SUCCESS

The test result readability is bad.
However it's ok.  You just followed the existing test.

> LayoutTests/fast/forms/checkValidity-001-expected.txt:10
> +PASS v[i].checkValidity() is true
> +PASS v[i].checkValidity() is true
> +PASS v[i].checkValidity() is true
>  PASS v[i].checkValidity() is true

ditto.

> LayoutTests/fast/forms/script-tests/setCustomValidity-existence.js:19
> +    + '</form>';
> +var controls = document.getElementsByName('victim');
> +    for (var i = 0; i < controls.length; i++)
> +        shouldBe('typeof controls[i].setCustomValidity', '"function"');

The indentation looks wrong.

The test result readability is not good.
shouldBe('document.getElementsByTagName("input")[0].setCustomValidity', '"function"');
shouldBe('document.getElementsByTagName("textarea")[0].setCustomValidity', '"function"');
...
is better.

> LayoutTests/fast/forms/willvalidate-expected.txt:51
> +Keygen element
> +FIXME: Let keygen.willValidate be false because <keygen> is barred from constraint validation.
> +FAIL document.getElementsByTagName("keygen")[0].willValidate should be false. Was true.

We should remove test cases for keygen, or should expand this bug so that it includes <keygen> fix.

[Kenichi Ishibashi]

Comment on attachment 75997 [details]
Patch V0

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

Kent-san,

Thank you for your prompt review. I'll post revised patch after the title of this bug changes.

>> LayoutTests/fast/forms/ValidityState-001-expected.txt:7
>>  SUCCESS
> 
> The test result readability is bad.
> However it's ok.  You just followed the existing test.

I've added tagName for each line.

>> LayoutTests/fast/forms/checkValidity-001-expected.txt:10
>>  PASS v[i].checkValidity() is true
> 
> ditto.

Modified to use the same representation that you suggested in the below comment.

>> LayoutTests/fast/forms/script-tests/setCustomValidity-existence.js:19
>> +        shouldBe('typeof controls[i].setCustomValidity', '"function"');
> 
> The indentation looks wrong.
> 
> The test result readability is not good.
> shouldBe('document.getElementsByTagName("input")[0].setCustomValidity', '"function"');
> shouldBe('document.getElementsByTagName("textarea")[0].setCustomValidity', '"function"');
> ...
> is better.

I've modified them following your suggestion. Thanks!

>> LayoutTests/fast/forms/willvalidate-expected.txt:51
>> +FAIL document.getElementsByTagName("keygen")[0].willValidate should be false. Was true.
> 
> We should remove test cases for keygen, or should expand this bug so that it includes <keygen> fix.

I'd like to include the fix for <keygen> in this bug, so could you change the title of this bug something like "Improve validation API support on <object> and <keygen>" or more appropriate one?

[Kenichi Ishibashi]

Created attachment 76011 [details]
Patch V1

[Kent Tamura]

Comment on attachment 76011 [details]
Patch V1

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

ok.

> LayoutTests/fast/forms/checkValidity-001.html:23
> +shouldBe('document.getElementsByTagName("fieldset")[0].checkValidity()', 'true');

nit: We can use shouldBeTrue().

[Kenichi Ishibashi]

Comment on attachment 76011 [details]
Patch V1

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

Kent-san,

Thanks for review!

>> LayoutTests/fast/forms/checkValidity-001.html:23
>> +shouldBe('document.getElementsByTagName("fieldset")[0].checkValidity()', 'true');
> 
> nit: We can use shouldBeTrue().

Thank you for letting me know that. I'll use it next time:-)

[WebKit Commit Bot]

Comment on attachment 76011 [details]
Patch V1

Rejecting patch 76011 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2
Last 500 characters of output:
octypes ............
fast/dom ......................................................................................................................................................................................................................................
fast/dom/plugin-attributes-enumeration.html -> failed

Exiting early after 1 failures. 6959 tests run.
131.47s total testing time

6958 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
3 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/6992007

[Kenichi Ishibashi]

Created attachment 76133 [details]
Patch V2

[Kenichi Ishibashi]

Fixed the expectation of fast/dom/plugin-attributes-enumeration.html.

(In reply to comment #9)
> Created an attachment (id=76133) [details]
> Patch V2

[Kent Tamura]

Comment on attachment 76133 [details]
Patch V2

ok

[WebKit Commit Bot]

Comment on attachment 76133 [details]
Patch V2

Clearing flags on attachment: 76133

Committed r73686: <http://trac.webkit.org/changeset/73686>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.