Opened 13 years ago

Closed 13 years ago

#7192 closed enhancement (fixed)

CDATA sections in XML data source are not correctly rendered in a grid

Reported by: niciliketo Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.2
Component: DojoX Data Version: 1.1.1
Keywords: dojox xml data cdata Cc: [email protected]…
Blocked By: Blocking:

Description

When trying to render XML to a grid, blanks are displayed when the XML is in a CDATA section. The CDATA sections is required because the content includes HTML tags. The problem appears to be in the Dojox.data.XmlStore? component, specifically the XmlItem? piece.

Attached are,

  1. an example of the XML I want to render to the grid.
  2. the code I am using
  3. A suggested modified version of the XmlData?.js file which does handle CDATA.

I have tested the issue and the fix against Firefox 3.0 and IE

Attachments (3)

XmlStore.js​ (33.5 KB) - added by niciliketo 13 years ago.
A version of the XmlStore?.js which I have changed to make CDATA work
data.xml​ (380 bytes) - added by niciliketo 13 years ago.
An example of data including CDATa
dojox.data.XmlStore_20080722.patch​ (6.6 KB) - added by Jared Jurkiewicz 13 years ago.
Quick patch + UT for adding CDATA support as part of the text handling.

Download all attachments as: .zip

Change History (13)

Changed 13 years ago by niciliketo

Attachment: XmlStore.js​ added

A version of the XmlStore?.js which I have changed to make CDATA work

Changed 13 years ago by niciliketo

Attachment: data.xml​ added

An example of data including CDATa

comment:1 Changed 13 years ago by Jared Jurkiewicz

Are you covered under CLA? If so, I can apply your changes fine.

comment:2 Changed 13 years ago by niciliketo

Hi Jared,

No, I am not. I can apply if this would be easier.

I am just an end user of Dojo, and don't pretend to understand everything that this change would affect. That is why it was just submitted as a suggestion.

Thanks,

Nic.

comment:3 Changed 13 years ago by Jared Jurkiewicz

I can't really look at your suggestions without you being under CLA (contamination issues). The reason is basically to avoid any possible questions of code pedigree. It's basically to protect dojo down the road, not a slight on the suggestion you made or you at all.

If you want to file for a CLA, that would be awesome. Would make it a lot faster to put a fix in for this or whatnot. :)

comment:4 Changed 13 years ago by Jared Jurkiewicz

And note: Suggestions being: Code you provide.

comment:5 in reply to:  4 Changed 13 years ago by niciliketo

Replying to jaredj:

And note: Suggestions being: Code you provide.

Understood, I will file a CLA later today and let you know once it is done. Thanks for your help.

comment:6 Changed 13 years ago by niciliketo

Hi Jared,

Sorry for the delay. I have now filed my CLA.

Thanks, Nic.

comment:7 Changed 13 years ago by Jared Jurkiewicz

Okay, cool. I'll take a look at your changes and if they seem safe, integrate them. Thanks for taking the time to do the initial work as well as fill out the CLA. Glad to have you as a contributor!

comment:8 Changed 13 years ago by Jared Jurkiewicz

Milestone: tbd → 1.2

Looking at this now. Found a couple places more than what you had that should have the CDATA check too ... so, applied them. Building a small testcase for it now off your supplied data file.

Should be able to get this in 1.2, so marking target as 1.2.

Changed 13 years ago by Jared Jurkiewicz

Attachment: dojox.data.XmlStore_20080722.patch​ added

Quick patch + UT for adding CDATA support as part of the text handling.

comment:9 Changed 13 years ago by Jared Jurkiewicz

Tested patch on:

Firefox 2.0.0.16

IE 6

Safari 3.1

Opera 9.2

Seamonkey 1.1.2

comment:10 Changed 13 years ago by Jared Jurkiewicz

Resolution: → fixed
Status: new → closed

Fixed in changeset: ​http://bugs.dojotoolkit.org/changeset/14549

Note: See TracTickets for help on using tickets.