Bug 50663 - Improve validation API support of <object> and <keygen>
Summary: Improve validation API support of <object> and <keygen>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 48821
Blocks: HTML5Forms
  Show dependency treegraph
 
Reported: 2010-12-07 19:38 PST by Kent Tamura
Modified: 2010-12-09 21:59 PST (History)
3 users (show)

See Also:


Attachments
Patch V0 (13.51 KB, patch)
2010-12-08 17:52 PST, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V1 (17.05 KB, patch)
2010-12-08 20:51 PST, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V2 (17.59 KB, patch)
2010-12-09 16:19 PST, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2010-12-07 19:38:57 PST
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-object-element

We should add
 - validity
 - validationMessage
 - checkValidity()
 - setCustomValidity()
Comment 1 Kenichi Ishibashi 2010-12-08 17:52:18 PST
Created attachment 75997 [details]
Patch V0
Comment 2 Kenichi Ishibashi 2010-12-08 17:56:36 PST
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
Comment 3 Kent Tamura 2010-12-08 18:17:43 PST
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.
Comment 4 Kenichi Ishibashi 2010-12-08 19:23:05 PST
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?
Comment 5 Kenichi Ishibashi 2010-12-08 20:51:08 PST
Created attachment 76011 [details]
Patch V1
Comment 6 Kent Tamura 2010-12-08 20:54:10 PST
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().
Comment 7 Kenichi Ishibashi 2010-12-08 20:59:54 PST
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:-)
Comment 8 WebKit Commit Bot 2010-12-09 04:13:17 PST
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
Comment 9 Kenichi Ishibashi 2010-12-09 16:19:25 PST
Created attachment 76133 [details]
Patch V2
Comment 10 Kenichi Ishibashi 2010-12-09 16:20:15 PST
Fixed the expectation of fast/dom/plugin-attributes-enumeration.html.

(In reply to comment #9)
> Created an attachment (id=76133) [details]
> Patch V2
Comment 11 Kent Tamura 2010-12-09 16:25:16 PST
Comment on attachment 76133 [details]
Patch V2

ok
Comment 12 WebKit Commit Bot 2010-12-09 21:59:49 PST
Comment on attachment 76133 [details]
Patch V2

Clearing flags on attachment: 76133

Committed r73686: <http://trac.webkit.org/changeset/73686>
Comment 13 WebKit Commit Bot 2010-12-09 21:59:55 PST
All reviewed patches have been landed.  Closing bug.