[webkit-reviews] review granted: [Bug 40429] REGRESSION: Can't submit a form with <input type=radio required> : [Attachment 58451] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 40429: REGRESSION: Can't submit a form with <input type=radio required>
https://bugs.webkit.org/show_bug.cgi?id=40429

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   Vector<RefPtr<Node> > buttons;
> +	   form()->getNamedElements(name(), buttons);

Why doesn't this function put its output in a Vector<RefPtr<Element> > or a
Vector<RefPtr<HTMLFormControlElement> >?

> +	   RefPtr<NodeList> nodeList = document()->getElementsByName(name());
> +	   if (nodeList) {

Why is the null check needed? Can getElementsByName return null?

Why use a NodeList? That is a data structure for the public DOM, but for
internal use I would recommend just writing a loop. The getElementsByName
function has no special algorithm and introduces unneeded overhead.

    for (Node* node = document(); node; node = node->traverseNextNode())

The above will do the same thing more efficiently than the NodeList code.

By the way, it worries me that we have to iterate every single item in the
whole document every time updateCheckedRadioButtons is called on a radio button
outside a form. Can't this get pathologically slow if there are a ton of radio
buttons?