[webkit-reviews] review denied: [Bug 38713] Support the labels attribute in labelable form controls : [Attachment 56449] Patch.

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Yael <yael.aharon at nokia.com>'s
request for review:
Bug 38713: Support the labels attribute in labelable form controls
https://bugs.webkit.org/show_bug.cgi?id=38713

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
WebCore/dom/Node.cpp:900
 +  void Node::notifyLocalNodeListsLabelChanged()
The content of this function is identical to
notifyLocalNodeListsChildrenChanged().
Don't you want to invalidate just NodeListsNodeData::m_labelsNodeListCache?



WebCore/dom/Node.cpp:2532
 +	    markDOMObjectWrapper(markStack, globalData,
nodeLists->m_labelsNodeListCache.get());
No tests for this behavior. 


WebCore/html/HTMLLabelElement.cpp:177
 +	    document()->notifyLocalNodeListsLabelChanged();
This part should be moved to parseMappedAttribute(). because we need to call it
for "for" HTML attribute change too.
We need to have tests for label.setAttribute("for", ...) and
label.removeAttribute("for").


LayoutTests/ChangeLog:8
 +	    * fast/form-controls: Added.
Why you need to have new directory?  Isn't fast/forms enough?


LayoutTests/ChangeLog:20
 +	    * fast/form-controls/labels-remove-parent-label.html: Added.
It seems we can write these tests in script-tests format.