Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

No CData #187

Merged
merged 3 commits into from
Dec 4, 2013
Merged

No CData #187

merged 3 commits into from
Dec 4, 2013

Conversation

mvrhov
Copy link

@mvrhov mvrhov commented Nov 27, 2013

I've defaulted the cdata to true to keep BC with existing serializer metadata. Although it's a bit annoying setting it to false on most of the string properties.

Added visitSimpleString into the XmlSerialization visitor, so Handlers can use that when serializing into the xml if they know that what is beeing serialized doesn0t need to be inside cdata

This one replaces #119

$doCData = $this->currentMetadata->xmlElementCData;
/*if ($this->currentMetadata->name == 'name') {
var_dump($this->currentMetadata);
}*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be removed

Miha Vrhovnik added 3 commits November 28, 2013 12:54
…t to be enclosed inside CDATA

Also fixed a bug where property metadata was not pushed onto the stack
…is method always serializes data into a text node.

DateTimeHandler supports that newly added feature.
schmittjoh added a commit that referenced this pull request Dec 4, 2013
@schmittjoh schmittjoh merged commit 22daa25 into schmittjoh:master Dec 4, 2013
@mvrhov mvrhov deleted the noCdata branch December 4, 2013 17:17
@Tobion
Copy link
Contributor

Tobion commented Mar 2, 2014

So when you set cdata=false but the string contains xml control characters, you get invalid xml?!

@Tobion
Copy link
Contributor

Tobion commented Mar 2, 2014

I think it makes more sense to have an option on the serialization context (similar to setSerializeNull()) to only add CDATA when it's needed, and not per-property.

@schmittjoh
Copy link
Owner

Have you tried this? I think any special characters will just get escaped.

@mvrhov
Copy link
Author

mvrhov commented Mar 2, 2014

IMO you get invalid XML. The reason why the text is not parsed is that parsing each node for invalid characters is really expensive in terms of performance.

@noetix
Copy link

noetix commented Feb 4, 2015

Without cdata annotation:

<FromSiteName>
    <![CDATA[ K & S Perth ]]>
</FromSiteName>

With cdata=false:

<FromSiteName>K & S Perth</FromSiteName>

But expected:

<FromSiteName>K &amp; S Perth</FromSiteName>

@mvrhov
Copy link
Author

mvrhov commented Feb 4, 2015

Yes, that's expected. please read above. You must specify cdata attribute where invalid xml text is expected.

@goetas goetas mentioned this pull request Mar 10, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants