334821 - CDATA handling is wrong

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

XPath states:
"Each character within a CDATA section is treated as character data. Thus, <![CDATA[<]]> in the source document will treated the same as &lt;. Both will result in a single < character in a text node in the tree. Thus, a CDATA section is treated as if the <![CDATA[ and ]]> were removed and every occurrence of < and & were replaced by &lt; and &amp; respectively."
[http://www.w3.org/TR/xpath#section-Text-Nodes]

That's not how we handle it right now.

Comment 1

[Allan Beaufour]

Attachment: Testcase

Comment 2

[awgrover]

Is this as a duplicate of 305242? CDATA markup in html/xhtml creates a dom comment node with leading text (approx) " CDATA ".

Comment 3

[Boris Zbarsky [:bzbarsky]]

This has nothing to do with bug 305242, since that bug is about tag-soup and this bug isn't really using tag-soup.

So has someone tried tracing the code to see where this goes wrong?

Comment 4

[Allan Beaufour]

(In reply to comment #3)
> This has nothing to do with bug 305242, since that bug is about tag-soup and
> this bug isn't really using tag-soup.
> 
> So has someone tried tracing the code to see where this goes wrong?

This is an XForms bug, we specifically search for text nodes only.

Comment 5

[Allan Beaufour]

Attachment: Patch

nsXFormsUtils::GetNodeValue() should treat text and cdata nodes equally.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Might I recommend one change?  Add all known types that don't have a string value defined explicitly in the switch.  Make the default case assert about an unknown node type.  This way when we add other node types to Gecko you'll be able to catch issues...

Comment 7

[Allan Beaufour]

(In reply to comment #6)
> Might I recommend one change?  Add all known types that don't have a string
> value defined explicitly in the switch.  Make the default case assert about an
> unknown node type.  This way when we add other node types to Gecko you'll be
> able to catch issues...

Good idea. Thx.

Comment 8

[Allan Beaufour]

Attachment: Patch v2

As bz suggested, explicitly check for node types. While I was at it:
* use EmptyString() instead of Truncate()
* initialize value to the empty string, in case something goes haywire in the GetNodeValue() calls
* do not return inside the switch. Maybe it's just me, but it hurts my eyes

Comment 9

[Allan Beaufour]

Fixed on trunk