Bug 80239

Summary: [Forms] The "legend" element should not be a form-associated element.
Product: WebKit Reporter: yosin
Component: FormsAssignee: yosin
Status: RESOLVED FIXED    
Severity: Normal CC: tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 80381    
Attachments:
Description Flags
Patch 1
none
Patch 2
none
Patch 3 none

Description yosin 2012-03-04 21:29:51 PST
According the specification, the "legend" element isn't a form-associate element.
http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#form-associated-element

However, current implementation of HTMLLegendElement is derived from HTMLFormControlElement. We have unexpected dependency between HTMLLegendElement and HTMLFormControlElement and FormAssociatedElement. Also, We waste time in loop of HTMLFormElement::m_associatedElements.

For clear understanding and better maintainability, it is better that C++ class hierarchy should be similar to specified in the specification.
Comment 1 yosin 2012-03-05 22:55:02 PST
Created attachment 130299 [details]
Patch 1
Comment 2 yosin 2012-03-05 22:56:20 PST
This bug is part of HTMLFormControlElement re-factoring bug https://bugs.webkit.org/show_bug.cgi?id=80381
Comment 3 Kent Tamura 2012-03-06 17:18:30 PST
Comment on attachment 130299 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=130299&action=review

The code change looks ok.

> Source/WebCore/ChangeLog:9
> +        This patch changes base class of HTMLLegendElement to HTMLElement from
> +        HTMLFormControlElement. 

You should mention a reason why we should change this.
Comment 4 yosin 2012-03-06 17:52:11 PST
Created attachment 130500 [details]
Patch 2
Comment 5 Kent Tamura 2012-03-06 17:58:02 PST
Comment on attachment 130500 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=130500&action=review

> Source/WebCore/ChangeLog:10
> +        This patch changes base class of HTMLLegendElement to HTMLElement from
> +        HTMLFormControlElement to match C++ class hierarchy to the specification
> +        and to be out from FormAssociatedElement class tree.

So, why we should follow the class hierarchy in the specification? Why HTMLLegendElement should not inherit from FormAssociatedElement?
You should say the benefit of the change.
Comment 6 yosin 2012-03-06 19:42:53 PST
Created attachment 130524 [details]
Patch 3
Comment 7 Kent Tamura 2012-03-06 19:50:41 PST
Comment on attachment 130524 [details]
Patch 3

ok
Comment 8 WebKit Review Bot 2012-03-07 00:09:32 PST
Comment on attachment 130524 [details]
Patch 3

Clearing flags on attachment: 130524

Committed r110029: <http://trac.webkit.org/changeset/110029>
Comment 9 WebKit Review Bot 2012-03-07 00:09:44 PST
All reviewed patches have been landed.  Closing bug.