Conversation
This fixes several things: - root nodes without children have to return false. - some_node->children() has to have children to be true. - child nodes without children are always true!
This already happens using simplexml_load_string, but it must also happen when directly instantiating and specifying the XML through constructor.
Perhaps this is not the best implementation. But at least it will allow us to access cdata.
Most public attributes can be literally copied to the cloned node. For attributes and children, they have to be regenerated.
- When using specifying a value to addChild, SimpleXML does not display it when the node is casted to a string, see the following test: hphp/test/slow/simple_xml/1641.php - However, when you cast the return value of this call to string, it does show a return value, see the following test: hphp/test/slow/simple_xml/1656.php (line 13) - Additionally, this fixes "string nesting" because m_children needed to be merged with the newly created string element due to the way strings work in this extension. Another test was added combining all these conditions.
Just like properties, when dealing with integer based lookups, we need to iterate ourselves and pick the appropriate child.
SimpleXML will implicitly convert string nodes to actual strings when the parent node is casted to an array. It will only do this for top level nodes. A test has been provided.
Once you cast a root element to an array, PHP will consider top level child nodes as new 'root nodes', these nodes behave differently to child nodes when it comes to boolean casting.
xmlIsBlankNode has to be called regardless of being a child or parent text node.
This allows the user to access a node's attribute through attributes()->some_attribute. This does not fix the fact that SimpleXML considers attributes to be actual SimpleXML nodes, rather than just strings. In practice this should not matter. Such a fix would deserve its own commit.
|
I want to do one final check on this which should happen sometime next week. It looks good so far though. |
|
Thanks for the update! |
|
Works perfectly for me, no more issues with SimpleXML -> thanks danslo! |
|
@danslo, we hit a problem with this, see https://gist.github.com/scannell/28a6319416738068fdcd. Is this something you can take a look at soon? |
|
@danslo: I reverted the specific part of this diff and added a new test in a diff that should hit Github soon. Please rename the two tests I turned off that affect Magento back to .php from .php.disabled and take a look -- the new test I added in that diff will indicate if this broke again. |
|
I think I've decided to port over php's SimpleXML. It might take a while (think weeks) but I'll come up with a cool PR. I'll make sure existing tests pass. |
Alright, let's try this again!
I've left out the commit that breaks the zend test. All commits below should include a test themselves.
Let me know if I need to change anything.