Bug 80239 - [Forms] The "legend" element should not be a form-associated element.
Summary: [Forms] The "legend" element should not be a form-associated element.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks: 80381
  Show dependency treegraph
 
Reported: 2012-03-04 21:29 PST by yosin
Modified: 2012-03-07 00:09 PST (History)
2 users (show)

See Also:


Attachments
Patch 1 (6.77 KB, patch)
2012-03-05 22:55 PST, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (6.87 KB, patch)
2012-03-06 17:52 PST, yosin
no flags Details | Formatted Diff | Diff
Patch 3 (6.95 KB, patch)
2012-03-06 19:42 PST, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.