[webkit-reviews] review denied: [Bug 34930] Implement interactive validation for forms : [Attachment 48728] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 22 08:26:06 PDT 2010


Adam Barth <abarth at webkit.org> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 34930: Implement interactive validation for forms
https://bugs.webkit.org/show_bug.cgi?id=34930

Attachment 48728: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=48728&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
This looks reasonable, but you need to spin up a new patch because this break
the Chromium build and there are few nits:

+ contorol
^^ Typo

+ unsigned i

We usually use int for these sorts of loops.

+ message.replace("%name", unhandled->name());

Please use String::format()

I'm also not super excited about adding a bunch of random arguments to these
functions.  Can we abstract this better somehow?  Also, the loop in
HTMLFormElement::prepareSubmit is pretty complicated, mostly just so we can
print more messages to the console.  Can we simplify this somehow?

Sorry for the delay in reviewing your patch (and the lame review).


More information about the webkit-reviews mailing list