Merge pull request #137 from apeschar/selfclosing

goetas · web-flow · commit c7c7657e3c2e · 2018-10-08T19:37:37.000+02:00
Respect self-closing tags only on foreign elements
diff --git a/src/HTML5/Parser/DOMTreeBuilder.php b/src/HTML5/Parser/DOMTreeBuilder.php
@@ -433,10 +433,16 @@ public function startTag($name, $attributes = array(), $selfClosing = false)
         else {
             $this->current->appendChild($ele);
 
-            // XXX: Need to handle self-closing tags and unary tags.
             if (! Elements::isA($name, Elements::VOID_TAG)) {
                 $this->current = $ele;
             }
+
+            // Self-closing tags should only be respected on foreign elements
+            // (and are implied on void elements)
+            // See: https://www.w3.org/TR/html5/syntax.html#start-tags
+            if (Elements::isHtml5Element($name)) {
+                $selfClosing = false;
+            }
         }
 
         // This is sort of a last-ditch attempt to correct for cases where no head/body
@@ -453,6 +459,11 @@ public function startTag($name, $attributes = array(), $selfClosing = false)
                 array_shift($this->nsStack);
             }
         }
+
+        if ($selfClosing) {
+            $this->endTag($name);
+        }
+
         // Return the element mask, which the tokenizer can then use to set
         // various processing rules.
         return Elements::element($name);
diff --git a/src/HTML5/Parser/Tokenizer.php b/src/HTML5/Parser/Tokenizer.php
@@ -383,11 +383,8 @@ protected function tagName()
         }
 
         $mode = $this->events->startTag($name, $attributes, $selfClose);
-        // Should we do this? What does this buy that selfClose doesn't?
-        if ($selfClose) {
-            $this->events->endTag($name);
-        } elseif (is_int($mode)) {
-            // fprintf(STDOUT, "Event response says move into mode %d for tag %s", $mode, $name);
+
+        if (is_int($mode)) {
             $this->setTextMode($mode, $name);
         }
 
diff --git a/test/HTML5/Parser/DOMTreeBuilderTest.php b/test/HTML5/Parser/DOMTreeBuilderTest.php
@@ -643,4 +643,40 @@ public function testSelectGroupedOptions()
         $this->assertSame(3, $dom->getElementById('first')->getElementsByTagName('option')->length);
         $this->assertSame(2, $dom->getElementById('second')->getElementsByTagName('option')->length);
     }
+
+    public function testVoidTag() {
+        $html = <<<EOM
+<!DOCTYPE html>
+<html>
+    <head>
+        <title>testVoidTag</title>
+        <meta>
+        <meta>
+    </head>
+    <body></body>
+</html>
+EOM;
+
+        $dom = $this->parse($html);
+        $this->assertSame(2, $dom->getElementsByTagName('meta')->length);
+        $this->assertSame(0, $dom->getElementsByTagName('meta')->item(0)->childNodes->length);
+        $this->assertSame(0, $dom->getElementsByTagName('meta')->item(1)->childNodes->length);
+    }
+
+    public function testIgnoreSelfClosingTag() {
+        $html = <<<EOM
+<!DOCTYPE html>
+<html>
+    <head>
+        <title>testIllegalSelfClosingTag</title>
+    </head>
+    <body>
+        <div /><span>Hello, World!</span></div>
+    </body>
+</html>
+EOM;
+
+        $dom = $this->parse($html);
+        $this->assertSame(1, $dom->getElementsByTagName('div')->item(0)->childNodes->length);
+    }
 }
diff --git a/test/HTML5/Parser/TokenizerTest.php b/test/HTML5/Parser/TokenizerTest.php
@@ -447,9 +447,10 @@ public function testSimpleTags()
         );
         foreach ($selfClose as $test => $expects) {
             $events = $this->parse($test);
-            $this->assertEquals(3, $events->depth(), "Counting events for '$test'" . print_r($events, true));
+            $this->assertEquals(2, $events->depth(), "Counting events for '$test'" . print_r($events, true));
             $this->assertEventEquals('startTag', $expects, $events->get(0));
-            $this->assertEventEquals('endTag', $expects, $events->get(1));
+            $event = $events->get(0);
+            $this->assertTrue($event['data'][2]);
         }
 
         $bad = array(
@@ -735,7 +736,7 @@ public function testTagAttributes()
                 true
             )
         );
-        $this->isAllGood('startTag', 3, $withEnd);
+        $this->isAllGood('startTag', 2, $withEnd);
 
         // Cause a parse error.
         $bad = array(

Original file line number	Diff line number	Diff line change
`@@ -383,11 +383,8 @@ protected function tagName()`
`383`	`383`	`}`
`384`	`384`
`385`	`385`	`$mode = $this->events->startTag($name, $attributes, $selfClose);`
`386`		`- // Should we do this? What does this buy that selfClose doesn't?`
`387`		`- if ($selfClose) {`
`388`		`- $this->events->endTag($name);`
`389`		`- } elseif (is_int($mode)) {`
`390`		`- // fprintf(STDOUT, "Event response says move into mode %d for tag %s", $mode, $name);`
	`386`	`+`
	`387`	`+ if (is_int($mode)) {`
`391`	`388`	`$this->setTextMode($mode, $name);`
`392`	`389`	`}`
`393`	`390`