[Webkit-unassigned] [Bug 38688] Support control attribute of HTMLLabelElement
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 10 16:51:13 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=38688
Darin Adler <darin at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55614|review? |review-
Flag| |
--- Comment #13 from Darin Adler <darin at apple.com> 2010-05-10 16:51:11 PST ---
(From update of attachment 55614)
> +static HTMLFormControlElement* nodeAsLabelableFormControl(Node* node)
> +{
> + if (!node)
> + return 0;
> +
> + if (node->isHTMLElement() && static_cast<HTMLElement*>(node)->isFormControlElement()) {
> + HTMLFormControlElement* formControlElement = static_cast<HTMLFormControlElement*>(node);
> + if (formControlElement->isLabelable())
> + return formControlElement;
> + }
> +
> + return 0;
> +}
I think this would read even better if it used early returns exclusively.
> + // Search the children of the label element for a form element.
This searches the descendants of the label element, not just the children.
> + if (HTMLFormControlElement* formControlElement = nodeAsLabelableFormControl(node))
> + return formControlElement;
As I said before, I see how this checks for "labelable" elements, but not "form-associated". Maybe that's the same thing as checking isFormControlElement?
> +class HTMLFormControlElement;
> +
> class HTMLLabelElement : public HTMLElement {
> public:
> HTMLLabelElement(const QualifiedName&, Document*);
> @@ -46,7 +48,7 @@ public:
> // Overridden to either click() or focus() the corresponding control.
> virtual void defaultEventHandler(Event*);
>
> - HTMLElement* correspondingControl();
> + HTMLElement* control();
Did you mean to change the type here to HTMLFormControlElement? If not, then why the forward declaration?
> + readonly attribute HTMLElement control;
Was the type here supposed to be HTMLFormControlElement?
I don't entirely understand why the manual test you made is a manual test. Automated tests can both activate and focus, although perhaps for hover you need a manual test.
I'm going to say review- mainly because of the unused forward declaration in HTMLLabelElement.h that clearly seems to be in error.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list