[webkit-reviews] review denied: [Bug 80110] [Forms] HTMLFieldSetElement.idl doesn't have elements attribute. : [Attachment 143537] proposed patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Rakesh <rakesh.kn at motorola.com>'s
request for review:
Bug 80110: [Forms] HTMLFieldSetElement.idl doesn't have elements attribute.
https://bugs.webkit.org/show_bug.cgi?id=80110

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143537&action=review


> Source/WebCore/html/HTMLFieldSetElement.h:64
> +    OwnPtr<HTMLFormCollection> m_elementsCollection;
> +
> +    mutable Vector<FormAssociatedElement*> m_associatedElements;
> +
> +    mutable uint64_t m_documentVersion;

nit: these blank lines are not needed.

m_documentVersion should be renamed to m_documentVersionForAssociatedElements,
or we should add a comment about the purpose.

> Source/WebCore/html/RadioNodeList.cpp:87
> +    return equalIgnoringCase(testElement->getIdAttribute(), m_name) ||
equalIgnoringCase(testElement->getNameAttribute(), m_name);

Please add a FIXME comment about the incorrectness, or fix the problem before
this bug.

> Source/WebCore/html/RadioNodeList.cpp:95
>  {
> +    if (testElement->hasTagName(objectTag))
> +	   return checkIfNameOrIdMatches(testElement);
> +
>      if (!testElement->isFormControlElement())

This change affects HTMLFormElement::elements too, right?
So, we should add <object> support before this bug.