[FIX]document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix in the tag name

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
DOM: Core & HTML
P2
normal
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Martin Honnen, Assigned: bz)

Tracking

({dev-doc-complete})

Trunk
mozilla1.9alpha1
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
When calling
  xmlDocument.getElementsByTagName('tagname')
Mozilla DOM implementation includes elements in the resulting NodeList which
have a prefix in the nodeName, e.g. elements alike
  <prefix:tagname xmlns:prefix="someURI" />
are included in the NodeList.
That is wrong in my view, and is also not the case in the Java DOM
implementation that comes with Sun JDK 1.4.
I will upload a test case consisting of an example XML documents that uses
namespaces and an HTML page that loads the XML documents and checks for certain
elements with xmlDocument.getElementsByTagName.
(Reporter)

Comment 1

14 years ago
Created attachment 123583 [details]
example XML document needed for test case
(Reporter)

Comment 2

14 years ago
Created attachment 123584 [details]
test case
(Reporter)

Comment 3

14 years ago
Explanation of test case output: for the XML

<gods xmlns="http://vatican.va/2003/gods">  
  <gods:god xmlns:gods="http://vatican.va/2003/gods">Kibo</gods:god>
  <god>Maho</god>
</gods>

the output is

xmlDocument.getElementsByTagName("god").length: 2
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods", "god").length: 2

while there is only one element with tag name "god", the other element has a tag
name of "gods:god".

For the XML

<gods xmlns="http://vatican.va/2003/gods">
  <gods:god xmlns:gods="http://vatican.va/2003/gods">Kibo</gods:god>
  <god>Maho</god>
  <god>Jaffo</god>
</gods>

the output is

xmlDocument.getElementsByTagName("god").length: 3
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods", "god").length: 3

while there are only two elements with tag name "god".

For the XML

<gods xmlns="http://vatican.va/2003/gods">
  <gods:god xmlns:gods="http://vatican.va/2003/gods">Kibo</gods:god>
  <god>Maho</god>
  <god>Jaffo</god>
  <aPrefix:god xmlns:aPrefix="http://vatican.va/2003/devils">Xibo</aPrefix:god>
</gods>

the output is

xmlDocument.getElementsByTagName("god").length: 4
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods", "god").length: 3

while there are only two elements with tag name "god", the other elements having
a tag name "gods:god" respectively "aPrefix:god".
(Reporter)

Comment 4

14 years ago
Created attachment 123586 [details]
comparable Java test case

This Java program is comparable to the HTML/JavaScript test case and shows the
difference in the implementation of xmlDocument.getElementsByTagName of Mozilla
and the Sun Java JDK 1.4. Both pretend to implement the W3C DOM Level 2 Core
and thus the results should be the same. I think the Java implementation is
correct and the Mozilla implementation needs to be fixed.

Here is the output of the Java program when run on the console with Win 98:

xmlDocument.getElementsByTagName("god").getLength(): 1
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods",
"god").getLeng
th(): 2
xmlDocument.getElementsByTagName("god").getLength(): 2
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods",
"god").getLeng
th(): 3
xmlDocument.getElementsByTagName("god").getLength(): 2
xmlDocument.getElementsByTagNameNS("http://vatican.va/2003/gods",
"god").getLeng
th(): 3
(Reporter)

Comment 5

14 years ago
Created attachment 123587 [details]
test case that checks tagName property of element with namespace prefix in the tag name
(Reporter)

Comment 6

14 years ago
The test case
http://bugzilla.mozilla.org/attachment.cgi?id=123587&action=view
shows that the problem with getElementsByTagName is probably caused by a wrong
value for the tagName property in Mozilla's DOM implementation. The W3C DOM
Level 2 Core specification in the description of createElementNS says that the
tagName should be the qualified name, it seems Mozilla wrongly sets the local
name as the tagName.
I have updated the bug summary to reflect this.
Summary: document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix → document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix in the tag name as the tagName property only contains the local name and not the qualified name
OS: Windows 98 → All
Hardware: PC → All
bz, this is due to nsContentList not knowing what prefix to match against, we'll
need some lowlevel nsContentListKey hacking here. Got some cycles?
Well, it's sounding like getElementsByTagName should just never find anything
that's not in the null namespace in XML docs... is that the case?  If so, this
should be pretty simple to do...
No, there's more to it than that. document.getElementsByTagName() should find
elements whose nodeName (e.g. "foo:bar") matches what's passed in, regardless of
the elements namespace (prefix or no prexix). I don't think this can be done w/o
holding on to a prefix one way or anyother in nsContentListKey.
Ah, I see what you mean.

nsContentListKey already holds on to a namespace id... We could simply change
the GetElementsByTagName code to look for a ':' in the tagname and convert it to
a namespace id and a localname if that happens, no?  If there is no ':', we use
the null namespace (HTML elements (not XHTML) report themselves as being in the
null namespace, right?)
No, unfortunately that won't work. The prefix can map to different namespaces in
different parts of the tree (XML namespaces can be redeclared). We need to know
the prefix and the namespace in nsContentListKey, but never both at the same
time (that I can think of). And yes, HTML elements are in the null namespace
(but make sure you ask the elements nodeinfo, not nsIContent::GetNameSpaceID()).
Oh, right...  Bleh.

Well, I won't have time to do this anytime soon (esp. what with the fact that
I'm still trying to be on vacation), but feel free to assign to me...
Flags: blocking1.9a2?
Created attachment 218838 [details]
Updated version of "test case" that should work with the new bugzilla URI
Attachment #123584 - Attachment is obsolete: true
Created attachment 218839 [details]
Update version of "tagName" testcase
Attachment #123587 - Attachment is obsolete: true
Created attachment 218840 [details] [diff] [review]
Fix

The tagName testcase already passes; this makes us pass the other testcase.  Note that this does change the behavior of getElementsByTagName (to follow spec).  Also note that this makes getElementsByTagName slower if it has to deal with elements with non-empty prefixes.  I think that's ok, myself.
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #218840 - Flags: superreview?(jst)
Attachment #218840 - Flags: review?(peterv)
Priority: -- → P2
Summary: document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix in the tag name as the tagName property only contains the local name and not the qualified name → [FIX]document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix in the tag name
Target Milestone: --- → mozilla1.9alpha
Attachment #218840 - Flags: review?(peterv) → review+
Comment on attachment 218840 [details] [diff] [review]
Fix

sr=jst
Attachment #218840 - Flags: superreview?(jst) → superreview+
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 18

11 years ago
searchWidgets.xml uses getElementsByTagName to iterate over anonymous xul:menuitem elements, should this now use the NS version?
Yeah.  Sorry I didn't catch that when I looked at in-tree uses.  :(

Comment 20

11 years ago
Created attachment 223159 [details] [diff] [review]
Fix for searchWidgets.xml

bz or anyone else, feel free to comment.
Attachment #223159 - Flags: superreview?(mscott)
Attachment #223159 - Flags: review?

Comment 21

11 years ago
Comment on attachment 223159 [details] [diff] [review]
Fix for searchWidgets.xml

thanks a lot for fixing this Neil.
Attachment #223159 - Flags: superreview?(mscott) → superreview+
Comment on attachment 223159 [details] [diff] [review]
Fix for searchWidgets.xml

Looks ok, but I'd probably just hardcode the XUL namespace, in case this binding gets applied to a random non-XUL element...
Attachment #223159 - Flags: review? → review+

Comment 23

11 years ago
(In reply to comment #22)
>(From update of attachment 223159 [details] [diff] [review])
>Looks ok, but I'd probably just hardcode the XUL namespace, in case this
>binding gets applied to a random non-XUL element...
In the unlikely event whomever is responsible can just deal :-P
How, exactly?  The point is, the code is conceptually wrong.  We really do want the XUL namespace, not the bound node namespace, esp. given xbl:extends.  Please make the change I asked for; the review is conditional on it.
Flags: blocking1.9a2?
Depends on: 343307
This caused bug 343307.

Comment 26

10 years ago
This has come up on the mailing list, so I mentioned this change on http://developer.mozilla.org/en/docs/DOM:element.getElementsByTagName

Probably worth a mention on the Firefox 3 page as well.
Flags: in-testsuite?
Keywords: dev-doc-needed
Do the tests in http://lxr.mozilla.org/seamonkey/source/content/test/unit/test_nodelist.js cover this being in a testsuite?

Comment 28

10 years ago
I think they do, yes: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/test/unit/test_nodelist.js&rev=&cvsroot=/cvsroot&mark=34-35,42-43#34
Flags: in-testsuite? → in-testsuite+
Added a reference to this issue to the Firefox 3 for developers page; tagging as doc complete.
Keywords: dev-doc-needed → dev-doc-complete

Updated

9 years ago
Duplicate of this bug: 414055

Updated

9 years ago
Component: DOM: Core → DOM: Core & HTML
QA Contact: desale → general
You need to log in before you can comment on or make changes to this bug.