Bug 58997 - Qualified names used for all TagName access, yet namespace usage is rare
Summary: Qualified names used for all TagName access, yet namespace usage is rare
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 08:50 PDT by Michael Saboff
Modified: 2011-04-21 18:42 PDT (History)
1 user (show)

See Also:


Attachments
Patch to use only local name when namespace is empty or "*" (8.48 KB, patch)
2011-04-20 09:16 PDT, Michael Saboff
mjs: review-
Details | Formatted Diff | Diff
Patch with updates to address reviewer's issues (8.69 KB, patch)
2011-04-21 05:29 PDT, Michael Saboff
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2011-04-20 08:50:37 PDT
Node::getElementsByTagName and Node::getElementsByTagNameNS always use qualified names yet few websites actually use namespaces.

Given that most access is without namespaces, will split into two paths.  A missing or "*" namespace can use the simple name instead of a fully qualified name.
Comment 1 Michael Saboff 2011-04-20 09:16:40 PDT
Created attachment 90347 [details]
Patch to use only local name when namespace is empty or "*"
Comment 2 Maciej Stachowiak 2011-04-20 12:33:54 PDT
Comment on attachment 90347 [details]
Patch to use only local name when namespace is empty or "*"

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>
Comment 3 Michael Saboff 2011-04-21 05:29:24 PDT
Created attachment 90513 [details]
Patch with updates to address reviewer's issues

Changed Node:getElementsByTagName to process namespace with implied "*" and changed Node::getElementsByTagNameNS to handle null namespaces.

No apparent impact on performance speed up.
Comment 4 Maciej Stachowiak 2011-04-21 17:55:41 PDT
Comment on attachment 90513 [details]
Patch with updates to address reviewer's issues

r=me
Comment 5 Michael Saboff 2011-04-21 18:42:16 PDT
Committed r84586: <http://trac.webkit.org/changeset/84586>