Bug 50663

Created attachment 75997 [details] Patch V0

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 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 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?

Created attachment 76011 [details] Patch V1

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 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 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

Created attachment 76133 [details] Patch V2

Fixed the expectation of fast/dom/plugin-attributes-enumeration.html. (In reply to comment #9) > Created an attachment (id=76133) [details] > Patch V2

Comment on attachment 76133 [details] Patch V2 ok

Comment on attachment 76133 [details] Patch V2 Clearing flags on attachment: 76133 Committed r73686: <http://trac.webkit.org/changeset/73686>

All reviewed patches have been landed. Closing bug.