MC-15022: HTML Content type does not render self closing html tags on storefront

danmooney2 · danmooney2 · commit f75dcdbffbda · 2019-02-22T09:45:49.000-06:00
Add comments and refactor
diff --git a/app/code/Magento/PageBuilder/Plugin/Filter/TemplatePlugin.php b/app/code/Magento/PageBuilder/Plugin/Filter/TemplatePlugin.php
@@ -143,27 +143,28 @@ private function decodeHtmlContentTypes(\DOMDocument $document): void
             // Store a decoded attribute on the element so we don't double decode
             $htmlContentTypeNode->setAttribute('data-decoded', 'true');
 
+            // if nothing exists inside the node, continue
             if (!strlen(trim($htmlContentTypeNode->nodeValue))) {
                 continue;
             }
 
+            // clone node and empty node value to prevent side-effects of modifying iterables in loop
             $clonedHtmlContentTypeNode = clone $htmlContentTypeNode;
             $clonedHtmlContentTypeNode->nodeValue = '';
 
             foreach ($htmlContentTypeNode->childNodes as $childNode) {
+                // if child node is not text node type, it does not need to be decoded; just append to cloned html node
                 if ($childNode->nodeType !== XML_TEXT_NODE) {
                     $clonedHtmlContentTypeNode->appendChild($childNode);
                     continue;
                 }
 
-                $fragDoc = new \DOMDocument();
-                $result = $fragDoc->loadHTML('<fragment>' . $childNode->nodeValue . '</fragment>');
+                // Load node value into dom document in an attempt to decode any encoded html within
+                $fragDoc = $this->createDomDocument($childNode->nodeValue);
 
-                if (!$result) {
-                    continue;
-                }
+                $import = $fragDoc->getElementsByTagName('body')->item(0);
 
-                $import = $fragDoc->getElementsByTagName('fragment')->item(0);
+                // Loop through decoded child nodes and import into the original document
                 foreach ($import->childNodes as $importedChildNode) {
                     $importedNode = $document->importNode($importedChildNode, true);
 
@@ -175,6 +176,7 @@ private function decodeHtmlContentTypes(\DOMDocument $document): void
                 }
             }
 
+            // replace original html content type node with cloned node we've been performing decoding operating on
             $htmlContentTypeNode->parentNode->replaceChild($clonedHtmlContentTypeNode, $htmlContentTypeNode);
         }
     }
diff --git a/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_post_filter.html b/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_post_filter.html
@@ -1,2 +1,3 @@
-<div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+<div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
 </div>
diff --git a/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_html_content_type_inside_post_filter.html b/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_html_content_type_inside_post_filter.html
@@ -1,3 +1,5 @@
-<div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
-    <div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+<div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+    <div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
 </div></div>
diff --git a/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_static_html_inside_post_filter.html b/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_static_html_inside_post_filter.html
@@ -1,3 +1,4 @@
-<div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+<div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
     <div>
     <img src="http://example.com/block"></div></div>
diff --git a/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_static_text_inside_post_filter.html b/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_with_block_with_static_text_inside_post_filter.html
@@ -1,3 +1,4 @@
-<div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+<div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
     Hello world
 </div>
diff --git a/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/text_content_type_with_block_with_html_content_type_inside_post_filter.html b/dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/text_content_type_with_block_with_html_content_type_inside_post_filter.html
@@ -1,5 +1,6 @@
 <div data-content-type="text">
     Hello there
-    <div data-content-type="html" data-decoded="true"><img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
+    <div data-content-type="html" data-decoded="true">
+    <img src="http://example.com"><iframe width="560" height="315" src="https://www.youtube.com/embed/owsfdh4gxyc" frameborder="0" allowfullscreen></iframe>
 </div>
 </div>

Original file line number	Diff line number	Diff line change
`@@ -143,27 +143,28 @@ private function decodeHtmlContentTypes(\DOMDocument $document): void`
`143`	`143`	`// Store a decoded attribute on the element so we don't double decode`
`144`	`144`	`$htmlContentTypeNode->setAttribute('data-decoded', 'true');`
`145`	`145`
	`146`	`+ // if nothing exists inside the node, continue`
`146`	`147`	`if (!strlen(trim($htmlContentTypeNode->nodeValue))) {`
`147`	`148`	`continue;`
`148`	`149`	`}`
`149`	`150`
	`151`	`+ // clone node and empty node value to prevent side-effects of modifying iterables in loop`
`150`	`152`	`$clonedHtmlContentTypeNode = clone $htmlContentTypeNode;`
`151`	`153`	`$clonedHtmlContentTypeNode->nodeValue = '';`
`152`	`154`
`153`	`155`	`foreach ($htmlContentTypeNode->childNodes as $childNode) {`
	`156`	`+ // if child node is not text node type, it does not need to be decoded; just append to cloned html node`
`154`	`157`	`if ($childNode->nodeType !== XML_TEXT_NODE) {`
`155`	`158`	`$clonedHtmlContentTypeNode->appendChild($childNode);`
`156`	`159`	`continue;`
`157`	`160`	`}`
`158`	`161`
`159`		`- $fragDoc = new \DOMDocument();`
`160`		`- $result = $fragDoc->loadHTML('<fragment>' . $childNode->nodeValue . '</fragment>');`
	`162`	`+ // Load node value into dom document in an attempt to decode any encoded html within`
	`163`	`+ $fragDoc = $this->createDomDocument($childNode->nodeValue);`
`161`	`164`
`162`		`- if (!$result) {`
`163`		`- continue;`
`164`		`- }`
	`165`	`+ $import = $fragDoc->getElementsByTagName('body')->item(0);`
`165`	`166`
`166`		`- $import = $fragDoc->getElementsByTagName('fragment')->item(0);`
	`167`	`+ // Loop through decoded child nodes and import into the original document`
`167`	`168`	`foreach ($import->childNodes as $importedChildNode) {`
`168`	`169`	`$importedNode = $document->importNode($importedChildNode, true);`
`169`	`170`
`@@ -175,6 +176,7 @@ private function decodeHtmlContentTypes(\DOMDocument $document): void`
`175`	`176`	`}`
`176`	`177`	`}`
`177`	`178`
	`179`	`+ // replace original html content type node with cloned node we've been performing decoding operating on`
`178`	`180`	`$htmlContentTypeNode->parentNode->replaceChild($clonedHtmlContentTypeNode, $htmlContentTypeNode);`
`179`	`181`	`}`
`180`	`182`	`}`