[webkit-reviews] review denied: [Bug 58997] Qualified names used for all TagName access, yet namespace usage is rare : [Attachment 90347] Patch to use only local name when namespace is empty or "*"

[bugzilla-daemon at webkit.org]

Maciej Stachowiak <mjs at apple.com> has denied Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 58997: Qualified names used for all TagName access, yet namespace usage is
rare
https://bugs.webkit.org/show_bug.cgi?id=58997

Attachment 90347: Patch to use only local name when namespace is empty or "*"
https://bugs.webkit.org/attachment.cgi?id=90347&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=90347&action=review

r-

> Source/WebCore/dom/Node.cpp:1736
> +    if (namespaceURI.isEmpty() || namespaceURI == starAtom)
> +	   return getElementsByTagName(localName);

I'm not sure that this is correct. getElementsByTagName is supposed to return
all elements with a given localName, so forwarding the "*" namespace URI to
getElementsByTagName is right. However, getElementsByTagName with an empty
namespace URI is only supposed to match elements with the given localName in
the null namespace. That is not the same behavior as getElementsByTagName.

Here is a test case that illustrates the difference:


<div id="testContainer">
</div>

<pre>
<script>
var testContainer = document.getElementById("testContainer");

var div1 = document.createElementNS(null, "div");
var div2 = document.createElementNS("http://madeupnamespace.com", "div");
testContainer.appendChild(div1);
testContainer.appendChild(div2);

document.writeln(testContainer.getElementsByTagName("div").length);
document.writeln(testContainer.getElementsByTagNameNS("*", "div").length);
document.writeln(testContainer.getElementsByTagNameNS(null, "div").length);
</script>
</pre>