Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Export is broken or contains empty fields (addChild does not escape & to &) #488

Open
mbo-s opened this issue Sep 17, 2018 · 8 comments

Comments

@mbo-s
Copy link

mbo-s commented Sep 17, 2018

Currently the Exportview uses the addChild method and this breaks, if the text contains an ampersand &

e.g. in the title
https://github.com/cross-solution/YAWIK/blob/develop/module/Jobs/view/jobs/export/feed.xml.phtml#L70

here the title is empty
<job id="5b3fc10b0acec3e060f8f502">
<title/>

expected value <title>Manager Sales &amp; Customer Relations</title>

What works is

$job->addChild('title');
$job->title = $jobObject->getTitle();

@cbleek
Copy link
Member

cbleek commented Nov 15, 2018

yes, you're right. It makes a difference.

cbleek@xenon:~$ php -a
Interactive mode enabled
php > $xml = simplexml_load_string('<jobopening/>');
php > $xml->addChild('title','a & b');
PHP Warning:  SimpleXMLElement::addChild(): unterminated entity reference               b in php shell code on line 1
PHP Stack trace:
PHP   1. {main}() php shell code:0
PHP   2. {main}() php shell code:0
PHP   3. SimpleXMLElement->addChild() php shell code:1
php > $xml->title='a & b';
php > $xml->saveXml();
php > echo $xml;
php > echo $xml->saveXml();
<?xml version="1.0"?>
<jobopening><title>a &amp; b</title></jobopening>

@cbleek
Copy link
Member

cbleek commented Nov 15, 2018

@TiSiE interessant. Scheint ein gewünschtes Feature zu sein. Wusstest du das?

https://stackoverflow.com/questions/552957/rationale-behind-simplexmlelements-handling-of-text-values-in-addchild-and-adda

@TiSiE
Copy link
Member

TiSiE commented Nov 15, 2018

Quick fix

replace all addChild('node', 'content') calls with $job->node = 'content'

Pragmatic fix

Extend SimpleXmlElement and make the addChild method behave like we want.

class MySimpleXmlElement extends \SimpleXmlElement
{
  public function addChild($node, $content = null, $escape = true)
  {
     if ($escape) {
       $this->$node = $content;
       return $this->$node;
     }

     return parent::addChild($node, $content);
  }
}

DRY and SOLID fix

Encapsulate the XML generation in a dedicated class. Maybe a job decorator similar to Jobs\Entity\Decorator\JsonLdProvider.
Or a filter to convert a job entity into a XML representation.

@cbleek
Copy link
Member

cbleek commented Nov 15, 2018

+1 DRY and SOLID fix

@TiSiE
Copy link
Member

TiSiE commented Nov 15, 2018

@sergey-galenko That's one for you, I suppose :)

@gastro24
Copy link

let's wait for @mbo-s opinion. They are the only ones currently using XML export.

@mbo-s
Copy link
Author

mbo-s commented Nov 16, 2018

@gastro24 I use the mentioned Quick Fix, so I do not need any change

@cbleek
Copy link
Member

cbleek commented Nov 16, 2018

Then we should set this onhold.

@TiSiE TiSiE added the pending label Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants