Bug 80499 – [Forms] label.form attribute doesn't work

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

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

[Kent Tamura]

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

[Kent Tamura]

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

[Kent Tamura]

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!

[Kent Tamura]

Comment on attachment 132210 [details]
Patch 4

ok

[WebKit Review Bot]

Comment on attachment 132210 [details]
Patch 4

Clearing flags on attachment: 132210

Committed r110996: <http://trac.webkit.org/changeset/110996>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.