[webkit-reviews] review denied: [Bug 28145] novalidate/formnovalidate support : [Attachment 35234] Patch v1

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 28145: novalidate/formnovalidate support
https://bugs.webkit.org/show_bug.cgi?id=28145

Attachment 35234: Patch v1
https://bugs.webkit.org/attachment.cgi?id=35234&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
These tests look pretty vacuous.  They would pass w/o any of the code changes
it seems.

+v = document.getElementsByName("victim");
+for (i = 0; i < v.length; i++) {
+    shouldBe("v[i].formNoValidate", "false");
+    v[i].formNoValidate = true;
+}
+for (i = 0; i < v.length; i++)
+    shouldBe("v[i].formNoValidate", "true");

Personally I prefer to write js-only tests, instead of making manual templates
as you have done.  Meaning, if you convert your form dom creation into JS
calls, this whole test can just live in a single .js file in resources/ and you
can use make-script-test-wrappers to generate the wrapper for you.

It seems it would be more interesting to test what getAttribute('novalidate')
gets set to when you change the JS properties.