[webkit-reviews] review denied: [Bug 19562] ValidityState object stub : [Attachment 22658] ValidityState stub class and related stuff, second revision

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Michelangelo De Simone
<m.des at mac.com>'s request for review:
Bug 19562: ValidityState object stub
https://bugs.webkit.org/show_bug.cgi?id=19562

Attachment 22658: ValidityState stub class and related stuff, second revision
https://bugs.webkit.org/attachment.cgi?id=22658&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
There are several instances of tabs instead of spaces.

I also don't think we should allocate a ValidityState object for every form
control.  They should be created on demand.

Your test case would be better if it printed the type of form control being
tested:
+Form Control #1 ValidityState exists - SUCCESS
+Form Control #1 customError
+Form Control #1 stepMismatch
+Form Control #1 valid
+Form Control #1 rangeUnderflow


And you only actually need to print on PASS or fail for each type of form
control.  Not one line for each validity state atribute.

also, all tests like this should use the fast/js .js template system.  See
examples in fast/js and other places where the entire test case is in
javascript and the wrapper html file is autogenerated.

Also, your test case is missing a newline at the end of the file:

+<ol id="console"></ol>
+</body>
+</html>
\ No newline at end of file
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog


r- for the memory issue.  These should be allocated on demand.

This is one of numerous examples of the fast/js javascript test template
system:
http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/DOMException/resource
s/EventException.js

http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/Document/resources/cr
eateElementNS-namespace-err.js
http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/Document/resources/TE
MPLATE.html