From 0f2dcaff89b9a3e9cd05a72a37c6372eeb25656d Mon Sep 17 00:00:00 2001 From: Andrey Maximov Date: Mon, 26 Oct 2015 17:18:32 +0300 Subject: [PATCH 1/5] Allow string of HTML without a root element in replaceNodeContent --- src/DomHelperTrait.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/DomHelperTrait.php b/src/DomHelperTrait.php index de72fd6..06727d0 100644 --- a/src/DomHelperTrait.php +++ b/src/DomHelperTrait.php @@ -85,25 +85,31 @@ protected function setNodeContent(\DOMNode $node, $content) { * A DOMNode object. * @param string $content * The text or HTML that will replace the contents of $node. + * + * @return array + * Array of DOMNode objects that replaced a node. */ protected function replaceNodeContent(\DOMNode &$node, $content) { if (strlen($content)) { - // Load the contents into a new DOMDocument and retrieve the element. - $replacement_node = Html::load($content)->getElementsByTagName('body') + // Load the contents into a new DOMDocument and retrieve elements. + $replacement_nodes = Html::load($content)->getElementsByTagName('body') ->item(0) - ->childNodes - ->item(0); + ->childNodes; } else { - $replacement_node = $node->ownerDocument->createTextNode(''); + $replacement_nodes = [$node->ownerDocument->createTextNode('')]; } // Import the updated DOMNode from the new DOMDocument into the original // one, importing also the child nodes of the replacement DOMNode. - $replacement_node = $node->ownerDocument->importNode($replacement_node, TRUE); - $node->parentNode->appendChild($replacement_node); + foreach ($replacement_nodes as &$replacement_node) { + $replacement_node = $node->ownerDocument->importNode($replacement_node, TRUE); + $node->parentNode->insertBefore($replacement_node, $node); + } $node->parentNode->removeChild($node); $node = $replacement_node; + + return $replacement_nodes; } /** From d7d04873ce5395ac7188d2466453073bffa64ac6 Mon Sep 17 00:00:00 2001 From: Andrey Maximov Date: Mon, 26 Oct 2015 17:18:54 +0300 Subject: [PATCH 2/5] Update tests for replaceNodeContent --- tests/src/Unit/DomHelperTraitTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/src/Unit/DomHelperTraitTest.php b/tests/src/Unit/DomHelperTraitTest.php index 17b75e3..f1f9193 100644 --- a/tests/src/Unit/DomHelperTraitTest.php +++ b/tests/src/Unit/DomHelperTraitTest.php @@ -72,6 +72,14 @@ public function testReplaceNodeContent() { // Test replacing again with a non-empty value. $this->replaceNodeContent($this->node, '
'); $this->assertEquals(Html::serialize($this->document), '
'); + // Test replacing again with a value without root element. + $new_nodes = $this->replaceNodeContent($this->node, '

first

second

'); + $this->assertEquals(Html::serialize($this->document), '

first

second

'); + $this->assertEquals(count($new_nodes), 2); + // Test replacing again with a non-empty value. + $new_nodes = $this->replaceNodeContent($this->node, '

third

'); + $this->assertEquals(Html::serialize($this->document), '

first

third

'); + $this->assertEquals(count($new_nodes), 1); } /** From aa2fde0354e74d42fa62ce7e308444fb322767bb Mon Sep 17 00:00:00 2001 From: Andrey Maximov Date: Mon, 26 Oct 2015 17:27:45 +0300 Subject: [PATCH 3/5] Do not use reference --- src/DomHelperTrait.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DomHelperTrait.php b/src/DomHelperTrait.php index 06727d0..0644011 100644 --- a/src/DomHelperTrait.php +++ b/src/DomHelperTrait.php @@ -102,12 +102,12 @@ protected function replaceNodeContent(\DOMNode &$node, $content) { // Import the updated DOMNode from the new DOMDocument into the original // one, importing also the child nodes of the replacement DOMNode. - foreach ($replacement_nodes as &$replacement_node) { + foreach ($replacement_nodes as $replacement_node) { $replacement_node = $node->ownerDocument->importNode($replacement_node, TRUE); $node->parentNode->insertBefore($replacement_node, $node); } $node->parentNode->removeChild($node); - $node = $replacement_node; + $node = end($replacement_nodes); return $replacement_nodes; } From 83e6e50b614038b97e7fec6f27fb959685749d7b Mon Sep 17 00:00:00 2001 From: Andrey Maximov Date: Mon, 26 Oct 2015 17:59:57 +0300 Subject: [PATCH 4/5] Fix tests and comments --- src/DomHelperTrait.php | 6 +++--- tests/src/Unit/DomHelperTraitTest.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/DomHelperTrait.php b/src/DomHelperTrait.php index 0644011..c87e499 100644 --- a/src/DomHelperTrait.php +++ b/src/DomHelperTrait.php @@ -86,8 +86,8 @@ protected function setNodeContent(\DOMNode $node, $content) { * @param string $content * The text or HTML that will replace the contents of $node. * - * @return array - * Array of DOMNode objects that replaced a node. + * @return \DOMNodeList + * DOMNodeList of DOMNode objects that replaced a node. */ protected function replaceNodeContent(\DOMNode &$node, $content) { if (strlen($content)) { @@ -107,7 +107,7 @@ protected function replaceNodeContent(\DOMNode &$node, $content) { $node->parentNode->insertBefore($replacement_node, $node); } $node->parentNode->removeChild($node); - $node = end($replacement_nodes); + $node = $replacement_node; return $replacement_nodes; } diff --git a/tests/src/Unit/DomHelperTraitTest.php b/tests/src/Unit/DomHelperTraitTest.php index f1f9193..abfd041 100644 --- a/tests/src/Unit/DomHelperTraitTest.php +++ b/tests/src/Unit/DomHelperTraitTest.php @@ -75,11 +75,11 @@ public function testReplaceNodeContent() { // Test replacing again with a value without root element. $new_nodes = $this->replaceNodeContent($this->node, '

first

second

'); $this->assertEquals(Html::serialize($this->document), '

first

second

'); - $this->assertEquals(count($new_nodes), 2); + $this->assertEquals($new_nodes->length, 2); // Test replacing again with a non-empty value. $new_nodes = $this->replaceNodeContent($this->node, '

third

'); $this->assertEquals(Html::serialize($this->document), '

first

third

'); - $this->assertEquals(count($new_nodes), 1); + $this->assertEquals($new_nodes->length, 1); } /** From eaf198bdeb451de936868f0fbf7d32913521a652 Mon Sep 17 00:00:00 2001 From: Andrey Maximov Date: Mon, 26 Oct 2015 20:54:30 +0300 Subject: [PATCH 5/5] Use array instead of DOMNodeList --- src/DomHelperTrait.php | 10 +++++++--- tests/src/Unit/DomHelperTraitTest.php | 11 ++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/DomHelperTrait.php b/src/DomHelperTrait.php index c87e499..f053a96 100644 --- a/src/DomHelperTrait.php +++ b/src/DomHelperTrait.php @@ -86,8 +86,8 @@ protected function setNodeContent(\DOMNode $node, $content) { * @param string $content * The text or HTML that will replace the contents of $node. * - * @return \DOMNodeList - * DOMNodeList of DOMNode objects that replaced a node. + * @return array + * Array of DOMNode objects that replaced a node. */ protected function replaceNodeContent(\DOMNode &$node, $content) { if (strlen($content)) { @@ -100,16 +100,20 @@ protected function replaceNodeContent(\DOMNode &$node, $content) { $replacement_nodes = [$node->ownerDocument->createTextNode('')]; } + // We can't just return $replacement_nodes because it may be an array or + // DOMNodeList object. So collect replacement nodes into new array. + $replacement_nodes_array = []; // Import the updated DOMNode from the new DOMDocument into the original // one, importing also the child nodes of the replacement DOMNode. foreach ($replacement_nodes as $replacement_node) { $replacement_node = $node->ownerDocument->importNode($replacement_node, TRUE); $node->parentNode->insertBefore($replacement_node, $node); + $replacement_nodes_array[] = $replacement_node; } $node->parentNode->removeChild($node); $node = $replacement_node; - return $replacement_nodes; + return $replacement_nodes_array; } /** diff --git a/tests/src/Unit/DomHelperTraitTest.php b/tests/src/Unit/DomHelperTraitTest.php index abfd041..c45ef6a 100644 --- a/tests/src/Unit/DomHelperTraitTest.php +++ b/tests/src/Unit/DomHelperTraitTest.php @@ -73,13 +73,18 @@ public function testReplaceNodeContent() { $this->replaceNodeContent($this->node, '
'); $this->assertEquals(Html::serialize($this->document), '
'); // Test replacing again with a value without root element. - $new_nodes = $this->replaceNodeContent($this->node, '

first

second

'); + $two_nodes = $this->replaceNodeContent($this->node, '

first

second

'); $this->assertEquals(Html::serialize($this->document), '

first

second

'); - $this->assertEquals($new_nodes->length, 2); + $this->assertEquals(count($two_nodes), 2); // Test replacing again with a non-empty value. $new_nodes = $this->replaceNodeContent($this->node, '

third

'); $this->assertEquals(Html::serialize($this->document), '

first

third

'); - $this->assertEquals($new_nodes->length, 1); + $this->assertEquals(count($new_nodes), 1); + // Test replacing of returned DOMNode objects. + $this->node = $two_nodes[0]; + $new_nodes = $this->replaceNodeContent($this->node, 'Second '); + $this->assertEquals(Html::serialize($this->document), 'Second

third

'); + $this->assertEquals(count($new_nodes), 3); } /**