Closed
Bug 6052
Opened 25 years ago
Closed 23 years ago
Node::nodeValue returning empty string instead of null
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla0.9.7
People
(Reporter: dbaron, Assigned: jst)
References
()
Details
(4 keywords, Whiteboard: [XPCDOM] relnote-devel)
Attachments
(2 files)
On an Element, Node::nodeValue is returning the empty string instead of null.
Run the "passive" tests on the above page.
I checked, and null is printing correctly, so it's not a toString or comparison
problem.
MSIE5 does get this correct.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M7
Comment 1•25 years ago
|
||
Hmm...you'll probably find that's true for all of the node types whose
nodeValue should be null.
I have a related bug that discussed how the idl->XPCOM+JS glue conversion
process converts DOMStrings to nsString references instead of nsString pointers.
Because of that, I can't return null for a string return type. This should
change (possibly in M7) since the nsIString interface is coming together.
Updated•25 years ago
|
Target Milestone: M7 → M10
Comment 2•25 years ago
|
||
Looks like there's still some contention internally over what the correct type
should be for strings in IDL. Pushing this one off till M10.
Comment 3•25 years ago
|
||
Can't solve this one till the IDL->XPCOM code generator stops using nsString
references. Adding the dependency in and leaving the M10 milestone, though I
suspect it may be pushed off till later.
Lots of upcoming string stuff in the DOM world, I hear, but I'm going to move
this to post-beta. brendan+vidur, does the [shared] wstring plan provide for a
way to fix this?
Target Milestone: M10 → M14
HTML DOM bugs are M11/P2 for Vidur.
Updated•25 years ago
|
Target Milestone: M11 → M15
Comment 6•25 years ago
|
||
I think shaver moved this one incorrectly to M11 (even though he meant
post-beta). Yes, the [shared] wstring plan will fix this problem. Moving to M15.
Comment 7•25 years ago
|
||
Since we need to wait for the nsIString work to happen first, this will need to
be pushed off.
Target Milestone: M15 → M17
Comment 8•24 years ago
|
||
This is also true for localName, namespaceURI and prefix. According to the
specification they should *always* return null for everything except Attr and
Element. Attr and Element should return null for everything created from the
Non-NS Level 2 methods.
localName has the problem that instead of returning null it is returning the
nodeName for newly created Attr, CDATASection, Comment, Text, DocumentFragment,
DocumentType, Element and ProcessingInstruction. Should this be a separate bug?
See http://www.mindspring.com/~bobclary/bugzilla/report3.html for a summary and
http://www.mindspring.com/~bobclary/TestFrame_dom_core.html for live tests (M17+
required)
Assignee | ||
Comment 9•24 years ago
|
||
Nominating for beta3, we must have a way to pass null strings through the DOM
APIs for beta3, should be trivial once scc fixes our string classes to support this.
Comment 10•24 years ago
|
||
Marking nsbeta3+ and re-assigning to jst.
Assignee: vidur → jst
Status: ASSIGNED → NEW
Whiteboard: [nsbeta3+]
Updated•24 years ago
|
Whiteboard: [nsbeta3+] → [nsbeta3+] [trivial fix]
Comment 11•24 years ago
|
||
PDT thinks that in order to justify a P2 priority, we'd like know what the user
impact of this ancient bug is. Does it really matter to users?
Whiteboard: [nsbeta3+] [trivial fix] → [nsbeta3+] [trivial fix][PDT needs info]
Comment 12•24 years ago
|
||
I believe this bug should be fixed, especially if it is trivial to do so.
First, the W3C DOM Core is the standard reference for everyone using the DOM.
Having Mozilla return a null string instead of null will result in continual
confusion now and in the future as more and more people code the DOM using the
W3C DOM Core specification. Additionally, this bug will show up where ever the
specification says a String value should return null, not just in nodeValue.
Namespace related attributes are an example. This bug is not restricted to the
DOM Core. It can appear anywhere in the DOM as well.
Second, conformance to the W3C DOM specification is important for cross-
platform scripting. I would like to be able to use the same code for Mozilla,
IE and server-side scripts. The attractiveness of the W3C DOM is that it is a
*standard* API. I don't want to get back into the situation where my scripts
have browser/platform checks scattered through out them... been there, done
that, no thanks!
Finally, I can't speak to the priority issue or to the effect fixing this bug
will have on existing code. I just wanted you to know that it is important to
me that Mozilla adhere to the standard as closely as possible. Given the choice
between a standards compliant browser and a non-standards compliant browser I
will choose the compliant browser everytime. But given the choice between two
non-standards compliant browsers, what choice do I make?
Assignee | ||
Comment 13•24 years ago
|
||
The root of this is a fundamental flaw in our string handling in the DOM
implementation, this affects a lot of widely used methods, such as
getAttribute(), this is not only a problem with Node::nodeValue. We're finally
close to having a string implementation that can handle this. Without this bug
fixed, existing pages that rely on *correct* DOM behavior will break!
Status: NEW → ASSIGNED
Comment 15•24 years ago
|
||
PDT thinks this is a P3, but if this is an issue on top100 sites, please let us
know right away.
Priority: P2 → P3
Whiteboard: [nsbeta3+] [trivial fix][PDT needs info] → [nsbeta3+] [trivial fix][PDTP3]
Comment 16•24 years ago
|
||
We will plan on fixing this in a future release. In the meantime, for developers
who wish to create content that complies with the intended functioning of the
DOM1 spec and works cross-browser with IE , the workaround is to
check whether you're running on Gecko via client sniffing, and insert a
conditional code fork in your JavaScript so that when you're running on Gecko,
you check the return value you get from executing this call, and if it's the
empty string, you manually change it to null.
Also, content developers should *not* write content that depends on the current
incorrect return value of empty string, as we will fix this bug in a future
release and such code would then break.
Keywords: relnote
Whiteboard: [nsbeta3+] [trivial fix][PDTP3] → [nsbeta3-] [trivial fix][PDTP3]
Target Milestone: M18 → Future
Comment 17•24 years ago
|
||
Adding helpwanted keyword. Maybe scc and I can get this done. Speaking from
bitter experience with the "level 0 DOM" (JS1.0, Nav 2.0, before there was a
standard, and still very much a de facto standard), telling people not to count
on functional behavior you ship is pretty much an exercise in futility.
/be
Keywords: helpwanted
Updated•24 years ago
|
Whiteboard: [nsbeta3-] [trivial fix][PDTP3] → [nsbeta3-] relnote-devel [trivial fix][PDTP3]
Comment 18•24 years ago
|
||
D'oh! I guess this missed Netscape 6 RTM cleanly. What's the status for
mozilla0.9 (for which I'm nominating)?
/be
Keywords: mozilla0.9
Assignee | ||
Comment 19•24 years ago
|
||
I (and jband) need scc's new string code to fix this but AFAIK that code is
close to done and once that's checked in this should be fairly easy to fix.
Aiming for mozilla0.9.
Priority: P3 → P2
Whiteboard: [nsbeta3-] relnote-devel [trivial fix][PDTP3] → [XPCDOM] relnote-devel
Target Milestone: Future → mozilla0.9
Comment 20•24 years ago
|
||
jst: per your request here are additional attributes that may be null.
Node.localName: for nodes other than Element or Attr localName should always be
null. Currently, localName contains other stuff (not ""). See
http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-NodeNSLocalN
Node.namespaceURI: for nodes other than Element or Attr namespaceURI should
always be null. Currently, namespaceURI is "". Note that the spec explicitly
says that namespaceURI == "" is different from namespaceURI == null. See
http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-NodeNSname
Node.prefix: See http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-NodeNSPrefix
Node.nodeValue: nodeValue can be null (see DocumentFragment) but currently is
reported as "". See http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-F68D080
Hope that helps.
bc
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9 → mozilla0.9.1
Updated•24 years ago
|
Component: DOM Level 1 → DOM Core
Comment 21•24 years ago
|
||
See bug #67876.
Comment 22•24 years ago
|
||
since the page on www.mindspring.com/~bobclary/ is no longer available,
attaching the test cases for localName, namespaceURI, nodeValue, prefix. Adding
testcase keyword.
Keywords: testcase
Comment 23•24 years ago
|
||
Comment 25•23 years ago
|
||
moving to TM of 0.9.2 per PDT triage (you can check it into 0.9.1 until Friday,
18/May/01 or into 0.9.2 after the tree opens)
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Assignee | ||
Comment 27•23 years ago
|
||
Moving to mozilla0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Updated•23 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Comment 28•23 years ago
|
||
I just tested this with 0.9.3 on a MacOS 9.1 box, and this still is a bug. It
fails on the tests provided by Netscape on the Node.localName,
Node.namespaceURI, Node.nodeValue, and Node.prefix in the Netscape test document
tc_cdot501. This test document is available here:
http://developer.netscape.com/evangelism/tools/testsuites/tc.html?txtTitle=DOM%20Core%20Level%202%20Conformance%20Tests&txtApiPath=w3c-dom-core-2/&txtTestCases=tc&txtInvariantApiTestFrame=tc_invariantTestFrame&txtMutantApiTestFrame=tc_mutantTestFrame
If this link doesn't work for you, try clicking on the DOM Core Level 2
Conformance test link on this page:
http://developer.netscape.com/evangelism/tools/testsuites/
Assignee | ||
Comment 29•23 years ago
|
||
Moving to mozilla0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Comment 30•23 years ago
|
||
Comment 31•23 years ago
|
||
The patch I attached fixes dbaron's tests. It does not fix every case where we
should return a null string in nsGenericDOMDataNode::GetNodeValue, because that
fix is more involved.
I'm more worried about the fallout from doing this change: what happened in
composer with getAttribute returning null might happen again with this patch. I
did check all the places where we get nodeValue, but I can't say for sure that
there won't be any problem. I did do some random browsing, mailing & composing
though and didn't see any problem.
Comment 32•23 years ago
|
||
Is this patch in the Oct 27 nightly build (2001102708)? If so, then it doesn't
fix the namespaceURI property bug for element nodes. Instead, it seems to
have changed the way the namespaceURI property is broken.
The correct behavior is described as follows:
If <foo> is the root element for a document, then you should have
<foo> -- node.namespaceURI=null (null for the namespaceURI)
<foo xmlns=""> -- node.namespaceURI="" (an 'empty' string for namespaceURI)
With the current build (2001102708) _both_ of the above produce a null
value for namespaceURI, which is incorrect.
Previously (Moz 0.9.5), the bug was different. In that case both examples
produce an 'empty' string value for namespaceURI, which is also wrong
(but in a different way).
Comment 33•23 years ago
|
||
I see. Indeed we always return a null string for namespaceURI on elements.
Should be easy to fix. Thanks.
Comment 34•23 years ago
|
||
[Additional hopefully useful comments]
Whatever is done, we should probably note the following weirdness
associated with it [From DOM 2 (core) spec, Sect 1,1,8, paragraph 3 -
maybe as a comment in the code referencing this location in the spec?]:
"Note that because the DOM does no lexical checking, the empty
string will be treated as a real namespace URI in DOM Level 2
methods. Applications must use the value null as the namespaceURI
parameter for methods if they wish to have no namespace."
The reason for this note is because the namespace specification
[Section 5.2] states the following:
"The default namespace can be set to the empty string. This has
the same effect, within the scope of the declaration, of there
being no default namespace."
So that although the DOM says that <foo> and <foo xmlns=""> are
different (one having namespaceURI=null and the other
namespaceURI=""), the namespace spec says that the two are
equivalent, and declare that there is 'no' default namespace.
This brings up the question of where NAMESPACE_ERR exceptions should
be thrown -- should they be thrown when parsing XML to form a DOM,
or only when manipulating using the DOM? Right now I don't think
these exceptions are ever thrown .... certainly I can use the DOM
to parse example documents that should throw NAMESPACE_ERR, such as
and of the following 3 examples::
<?xml version="1.0" ?>
<!-- This should fail - you're not allowed to declare
a no-namespace value for a prefixed attribute namespace declaration
like xmlns:foo -->
<xmltest xmlns:foo=""
xmlns:html="http://www.w3.org/TR/REC-html40"
foo:name="value" >
<html:script src="tree-traverse.js" />
</xmltest>
<?xml version="1.0" ?>
<!-- This should fail - the two prefixes map onto the same
namespace, so the two attributes conflict.
-->
<xmltest xmlns:abc="http://www.foo.org"
xmlns:html="http://www.w3.org/TR/REC-html40"
xmlns:foo="http://www.foo.org"
foo:name="value"
abc:name="value2">
<html:script src="tree-traverse.js" />
</xmltest>
<?xml version="1.0" ?>
<!-- This should raise a NAMESPACE_ERR exception because
the Qname foo:name uses a prefix that has no declared namespace" -->
<xmltest foo:name="value" xmlns:html="http://www.w3.org/TR/REC-html40" >
<html:script src="tree-traverse.js" />
</xmltest>
Assignee | ||
Comment 35•23 years ago
|
||
Pushing to mozilla0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Assignee | ||
Comment 36•23 years ago
|
||
Fixed, please open a new bug on the element.namespaceURI issues.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 37•23 years ago
|
||
verified fixed, all the tests provided passed. build 2001-12-14-09
Status: RESOLVED → VERIFIED
Component: DOM: Core → DOM: Core & HTML
QA Contact: stummala → general
You need to log in
before you can comment on or make changes to this bug.
Description
•