[webkit-reviews] review denied: [Bug 101044] HTMLLegendElement.form should be null when it does not have a fieldset element as parent : [Attachment 172083] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 101044: HTMLLegendElement.form should be null when it does not have a
fieldset element as parent
https://bugs.webkit.org/show_bug.cgi?id=101044

Attachment 172083: Patch
https://bugs.webkit.org/attachment.cgi?id=172083&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=172083&action=review


I don't decide whether this change should be landed or not. Anyway, the code is
wrong.

> LayoutTests/fast/forms/form-attribute.html:88
> +debug('- Ensures that the form attribute of legend element depends on
whether it is in a fieldset element or not.');
> +container.innerHTML = '<form id=owner>' +
> +    '    <fieldset><legend id=legendElement1 name=victim /></fieldset>' +
> +    '    <legend id=legendElement2 name=victim />' +
> +    '</form>';
> +owner = document.getElementById('owner');
> +var legendElement1 = document.getElementById('legendElement1');
> +var legendElement2 = document.getElementById('legendElement2');
> +shouldBe('legendElement1.form', 'owner');
> +shouldBe('legendElement2.form', 'null');
> +
> +debug('');

Please do not add tests to this file.  form-attribute.html is a test file for
"form" content attribute.
You should make LayoutTests/fast/forms/legend/legend-form.html

> Source/WebCore/html/HTMLLegendElement.cpp:94
> +    // As per the specification, we should return null if the
> +    // legend does not have a fieldset element as its parent.
> +    if (!parentNode()->hasTagName(fieldsetTag))
> +	   return 0;
> +
> +    return findFormAncestor();

parentNode() can be 0.
Also, this doesn't match to the specification, which says "must return the same
value as the form IDL attribute on that fieldset element."
If the parent <fieldset> has "form" content attribute, <fieldset>'s form is not
the ancestor form.

> Source/WebCore/html/HTMLLegendElement.h:45
> +    virtual HTMLFormElement* virtualForm() const;

Need to append OVERRIDE.