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)

defect

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.
Status: NEW → ASSIGNED
Target Milestone: M7
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.
Target Milestone: M7 → M10
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.
Blocks: 1994
Depends on: 1663
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.
Target Milestone: M11 → M15
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.
Since we need to wait for the nsIString work to happen first, this will need to be pushed off.
Target Milestone: M15 → M17
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)
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.
Keywords: correctness, nsbeta3
OS: other → All
Hardware: Other → All
Target Milestone: M17 → M18
Marking nsbeta3+ and re-assigning to jst.
Assignee: vidur → jst
Status: ASSIGNED → NEW
Whiteboard: [nsbeta3+]
Whiteboard: [nsbeta3+] → [nsbeta3+] [trivial fix]
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]
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?
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
Mass update of qa contact
QA Contact: gerardok → janc
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]
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
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
Whiteboard: [nsbeta3-] [trivial fix][PDTP3] → [nsbeta3-] relnote-devel [trivial fix][PDTP3]
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
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
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
Target Milestone: mozilla0.9 → mozilla0.9.1
Keywords: dom1
Component: DOM Level 1 → DOM Core
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
QA contact Update
QA Contact: janc → desale
Blocks: 75185
No longer blocks: 1994
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
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala
Moving to mozilla0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla0.9.4
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/
Moving to mozilla0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Target Milestone: mozilla0.9.5 → mozilla0.9.6
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.
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).
I see. Indeed we always return a null string for namespaceURI on elements. Should be easy to fix. Thanks.
[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>
Pushing to mozilla0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Fixed, please open a new bug on the element.namespaceURI issues.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: