The default bug view has changed. See this FAQ.

Status

Core Graveyard
XForms
RESOLVED FIXED
11 years ago
8 months ago

People

(Reporter: Allan Beaufour, Assigned: Allan Beaufour)

Tracking

({fixed1.8.0.5, fixed1.8.1})

Trunk
fixed1.8.0.5, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

1.06 KB, application/xhtml+xml
Details
2.67 KB, patch
smaug
: review+
aaronr
: review+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
Created attachment 219143 [details]
Testcase

Comment 2

11 years ago
Is this as a duplicate of 305242? CDATA markup in html/xhtml creates a dom comment node with leading text (approx) " CDATA ".
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?
(Assignee)

Comment 4

11 years ago
(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.
(Assignee)

Comment 5

11 years ago
Created attachment 220787 [details] [diff] [review]
Patch

nsXFormsUtils::GetNodeValue() should treat text and cdata nodes equally.
Assignee: aaronr → allan
Status: NEW → ASSIGNED
Attachment #220787 - Flags: review?(Olli.Pettay)

Updated

11 years ago
Attachment #220787 - Flags: review?(aaronr)
Attachment #220787 - Flags: review?(Olli.Pettay)
Attachment #220787 - Flags: review+
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...
(Assignee)

Comment 7

11 years ago
(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.
(Assignee)

Updated

11 years ago
Attachment #220787 - Flags: review?(aaronr)
(Assignee)

Comment 8

11 years ago
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
Attachment #220787 - Attachment is obsolete: true
Attachment #220794 - Flags: review?(Olli.Pettay)

Updated

11 years ago
Attachment #220794 - Flags: review?(aaronr)
Attachment #220794 - Flags: review?(Olli.Pettay)
Attachment #220794 - Flags: review+

Updated

11 years ago
Attachment #220794 - Flags: review?(aaronr) → review+
(Assignee)

Comment 9

11 years ago
Fixed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.0.5
(Assignee)

Updated

11 years ago
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.