Bug 87371

Created attachment 143816 [details] patch

Comment on attachment 143816 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=143816&action=review > Source/WebCore/html/RadioNodeList.cpp:93 > + if (testElement->hasTagName(objectTag)) > + return checkIfNameOrIdMatches(testElement); Would you add an assertion that ASSERT(m_formElement == static_cast<HTMLObjectElement*>(testElement)->form()); please?

(In reply to comment #2) > (From update of attachment 143816 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143816&action=review > Thanks for r+. > > Source/WebCore/html/RadioNodeList.cpp:93 > > + if (testElement->hasTagName(objectTag)) > > + return checkIfNameOrIdMatches(testElement); > > Would you add an assertion that > ASSERT(m_formElement == static_cast<HTMLObjectElement*>(testElement)->form()); > > please? I think we should check this condition in 'if' here as the root node for DynamicSubTreeNodeList is document() and sub tree can contain other object elements whose form owner is not this form. And this is applicable only for form.elements and not fieldset.elements. Please let me know your thoughts on this.

Comment on attachment 143816 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=143816&action=review >>> Source/WebCore/html/RadioNodeList.cpp:93 >>> + return checkIfNameOrIdMatches(testElement); >> >> Would you add an assertion that >> ASSERT(m_formElement == static_cast<HTMLObjectElement*>(testElement)->form()); >> >> please? > > I think we should check this condition in 'if' here as the root node for DynamicSubTreeNodeList is document() and sub tree can contain other object elements whose form owner is not this form. And this is applicable only for form.elements and not fieldset.elements. > Please let me know your thoughts on this. Oh, I see. I thought testElement came from HTMLFormElement::m_associatedElements. So we should check form(). How about moving the form() check into checkIfNameOrIdMatches(), and rename it to an appropriate name?

Created attachment 143990 [details] patch Added form check for object element also.

Comment on attachment 143990 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=143990&action=review > Source/WebCore/html/RadioNodeList.cpp:103 > + if (!testElement->hasTagName(objectTag)) { > + if (!testElement->isFormControlElement()) nit: this can be if (!testElement->hasTagName(objectTag) && !testElement->isFormControlElement()) return false;

Created attachment 143994 [details] patch Patch for landing.

Comment on attachment 143994 [details] patch Clearing flags on attachment: 143994 Committed r118495: <http://trac.webkit.org/changeset/118495>

All reviewed patches have been landed. Closing bug.