Last Comment Bug 334821 - CDATA handling is wrong
: CDATA handling is wrong
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Allan Beaufour
: Stephen Pride
Mentors:
http://www.w3.org/TR/xforms/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-20 06:39 PDT by Allan Beaufour
Modified: 2016-07-15 14:46 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Testcase (1.06 KB, application/xhtml+xml)
2006-04-20 06:40 PDT, Allan Beaufour
no flags Details
Patch (1.62 KB, patch)
2006-05-04 08:36 PDT, Allan Beaufour
bugs: review+
Details | Diff | Splinter Review
Patch v2 (2.67 KB, patch)
2006-05-04 09:11 PDT, Allan Beaufour
bugs: review+
aaronr: review+
Details | Diff | Splinter Review

Description Allan Beaufour 2006-04-20 06:39:42 PDT
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 2006-04-20 06:40:16 PDT
Created attachment 219143 [details]
Testcase
Comment 2 awgrover 2006-05-04 07:07:38 PDT
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 [:bz] 2006-05-04 07:47:14 PDT
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 2006-05-04 08:32:34 PDT
(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 2006-05-04 08:36:11 PDT
Created attachment 220787 [details] [diff] [review]
Patch

nsXFormsUtils::GetNodeValue() should treat text and cdata nodes equally.
Comment 6 Boris Zbarsky [:bz] 2006-05-04 08:50:50 PDT
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 2006-05-04 08:53:30 PDT
(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 2006-05-04 09:11:59 PDT
Created attachment 220794 [details] [diff] [review]
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 2006-05-12 01:13:23 PDT
Fixed on trunk

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