Last Comment Bug 206053 - [FIX]document.getElementsByTagName('tagname') with XML document wrongly includes elements with namespace prefix in the tag name
: [FIX]document.getElementsByTagName('tagname') with XML document wrongly inclu...
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
: 414055 (view as bug list)
Depends on: 343307
Blocks:
  Show dependency treegraph
 
Reported: 2003-05-17 06:44 PDT by Martin Honnen
Modified: 2008-07-31 02:36 PDT (History)
6 users (show)
asqueella: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
example XML document needed for test case (182 bytes, text/xml)
2003-05-17 06:45 PDT, Martin Honnen
no flags Details
test case (2.57 KB, text/html)
2003-05-17 06:48 PDT, Martin Honnen
no flags Details
comparable Java test case (2.61 KB, text/plain)
2003-05-17 07:11 PDT, Martin Honnen
no flags Details
test case that checks tagName property of element with namespace prefix in the tag name (1014 bytes, text/html)
2003-05-17 07:33 PDT, Martin Honnen
no flags Details
Updated version of "test case" that should work with the new bugzilla URI (2.57 KB, text/html)
2006-04-18 10:04 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details
Update version of "tagName" testcase (1015 bytes, text/html)
2006-04-18 10:07 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details
Fix (8.95 KB, patch)
2006-04-18 10:11 PDT, Boris Zbarsky [:bz] (still a bit busy)
peterv: review+
jst: superreview+
Details | Diff | Splinter Review
Fix for searchWidgets.xml (3.88 KB, patch)
2006-05-24 02:31 PDT, neil@parkwaycc.co.uk
bzbarsky: review+
mscott: superreview+
Details | Diff | Splinter Review

Description Martin Honnen 2003-05-17 06:44:16 PDT
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.
Comment 1 Martin Honnen 2003-05-17 06:45:54 PDT
Created attachment 123583 [details]
example XML document needed for test case
Comment 2 Martin Honnen 2003-05-17 06:48:33 PDT
Created attachment 123584 [details]
test case
Comment 3 Martin Honnen 2003-05-17 07:01:32 PDT
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".
Comment 4 Martin Honnen 2003-05-17 07:11:15 PDT
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
Comment 5 Martin Honnen 2003-05-17 07:33:53 PDT
Created attachment 123587 [details]
test case that checks tagName property of element with namespace prefix in the tag name
Comment 6 Martin Honnen 2003-05-17 07:42:42 PDT
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.
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2003-05-19 18:26:11 PDT
bz, this is due to nsContentList not knowing what prefix to match against, we'll
need some lowlevel nsContentListKey hacking here. Got some cycles?
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2003-05-19 18:42:16 PDT
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...
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2003-05-19 19:52:07 PDT
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.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2003-05-19 20:35:05 PDT
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?)
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2003-05-19 22:55:20 PDT
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()).
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2003-05-20 07:53:20 PDT
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...
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2006-04-18 10:04:49 PDT
Created attachment 218838 [details]
Updated version of "test case" that should work with the new bugzilla URI
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2006-04-18 10:07:04 PDT
Created attachment 218839 [details]
Update version of "tagName" testcase
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2006-04-18 10:11:06 PDT
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.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2006-04-24 21:40:23 PDT
Comment on attachment 218840 [details] [diff] [review]
Fix

sr=jst
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2006-04-25 14:51:21 PDT
Fixed.
Comment 18 neil@parkwaycc.co.uk 2006-05-23 16:47:06 PDT
searchWidgets.xml uses getElementsByTagName to iterate over anonymous xul:menuitem elements, should this now use the NS version?
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2006-05-23 18:30:37 PDT
Yeah.  Sorry I didn't catch that when I looked at in-tree uses.  :(
Comment 20 neil@parkwaycc.co.uk 2006-05-24 02:31:24 PDT
Created attachment 223159 [details] [diff] [review]
Fix for searchWidgets.xml

bz or anyone else, feel free to comment.
Comment 21 Scott MacGregor 2006-05-24 08:50:02 PDT
Comment on attachment 223159 [details] [diff] [review]
Fix for searchWidgets.xml

thanks a lot for fixing this Neil.
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2006-05-27 08:35:50 PDT
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...
Comment 23 neil@parkwaycc.co.uk 2006-05-27 13:59:32 PDT
(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
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2006-05-29 11:44:55 PDT
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.
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2006-06-30 22:57:19 PDT
This caused bug 343307.
Comment 26 Nickolay_Ponomarev 2007-03-19 05:46:24 PDT
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.
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2007-03-30 11:40:33 PDT
Do the tests in http://lxr.mozilla.org/seamonkey/source/content/test/unit/test_nodelist.js cover this being in a testsuite?
Comment 29 Eric Shepherd [:sheppy] 2007-10-02 09:44:53 PDT
Added a reference to this issue to the Firefox 3 for developers page; tagging as doc complete.
Comment 30 Foteos Macrides 2008-01-26 12:53:50 PST
*** Bug 414055 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.