According to the standard[1], the "label" element has "form" attribute in IDL. However, in sample URI(http://jsfiddle.net/gDcAG/3/), JS expression label1.form returns null instead of HTMLFormElement object, even if HTMLLabelElement.idl[2] has "form" attribute. "form" attribute is implemented in HTMLElement.h[3] as virtualForm/findFormAncestor in HTMLElement.cpp[4]. In this implementation, "label" elements outside "form" elements don't work as expected. == References == [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#the-label-element [2] http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLLabelElement.idl [3] http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.h [4] http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLElement.cpp
Created attachment 131799 [details] Patch 1
Comment on attachment 131799 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=131799&action=review > Source/WebCore/html/HTMLLabelElement.cpp:88 > +{ The code is very similar to FormAssociated::insertedIntoTree() and FormAssociated::resetFormOwner(). Please share the code. > LayoutTests/fast/forms/form-attribute-expected.txt:NaN > PASS document.getElementsByTagName("button")[0].form is owner Please remove FIXME in this file. > LayoutTests/fast/forms/label/label-form-attribute.html:1 > +<DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> As you know, we already have some tests for 'form' attribute. I recommend add test cases for 'label' to form-attribute.html and form-attribute-nonexistence-form-id.html, and remove this file. > LayoutTests/fast/forms/label/label-form-attribute.html:12 > +var html = '<dvi id=outer>'; typo: <dvi>
Created attachment 131817 [details] Patch 2
Comment on attachment 131817 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=131817&action=review Please rebase the patch. http://trac.webkit.org/changeset/110918 conflicts with this. > Source/WebCore/html/FormAssociatedElement.cpp:100 > + HTMLElement* element = toHTMLElement(this); > + setForm(findAssociatedForm(element, m_form)); The variable "element" is not needed. setForm(findAssociatedForm(toHTMLElement(this), m_form)); > Source/WebCore/html/FormAssociatedElement.cpp:155 > + setForm(findAssociatedForm(element, m_form)); ditto.
Created attachment 132187 [details] Patch 3
Comment on attachment 132187 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=132187&action=review > Source/WebCore/html/HTMLLabelElement.cpp:33 > +#include "HTMLFormControlElement.h" > +#include "HTMLFormElement.h" Do we need to include HTMLFormControlElement.h and HTMLFormElement.h?
Created attachment 132210 [details] Patch 4
(In reply to comment #6) > (From update of attachment 132187 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132187&action=review > > > Source/WebCore/html/HTMLLabelElement.cpp:33 > > +#include "HTMLFormControlElement.h" > > +#include "HTMLFormElement.h" > > Do we need to include HTMLFormControlElement.h and HTMLFormElement.h? Removed. Thanks for catching this!
Comment on attachment 132210 [details] Patch 4 ok
Comment on attachment 132210 [details] Patch 4 Clearing flags on attachment: 132210 Committed r110996: <http://trac.webkit.org/changeset/110996>
All reviewed patches have been landed. Closing bug.