Repository: ampproject/amp-toolbox-php (php)

Issue:
`SelfClosingSVGElements` document filter erroneously self-closes `g` tags
As reported in a AMP plugin [support forum topic](https://wordpress.org/support/topic/svg-group-tag-is-broken/), given this input HTML:

```html
<svg>
  <g id="ok">
    <path/>
  </g>
</svg>
```

The output is:

```html
<svg>
  <g id="ok"/>
    <path/>
</svg>
```

I was able to prevent this from happening by commenting out `Tag::G` from `SelfClosingSVGElements::SELF_CLOSING_TAGS`:

https://github.com/ampproject/amp-toolbox-php/blob/4cecbc5953823f15c2f9ea22ec3ee947040c1547/src/Dom/Document/Filter/SelfClosingSVGElements.php#L21-L25

Pull request:
Prevent `SelfClosingSVGElements` document filter from erroneously self-closing tags
Fixes #512

Ensures that `SelfClosingSVGElements` will properly process:

* SVG DOM trees that contain newlines.
* SVG self-closing that have no whitespace (e.g. `<path/>` as opposed to `<path />`).

Patch:
diff --git a/src/Dom/Document/Filter/SelfClosingSVGElements.php b/src/Dom/Document/Filter/SelfClosingSVGElements.php
index 8fbae51b0..fa2e82fb5 100644
--- a/src/Dom/Document/Filter/SelfClosingSVGElements.php
+++ b/src/Dom/Document/Filter/SelfClosingSVGElements.php
@@ -35,7 +35,7 @@ public function beforeLoad($html)
         static $regexPattern = null;
 
         if (null === $regexPattern) {
-            $regexPattern = '#<(' . implode('|', self::SELF_CLOSING_TAGS) . ')((?>\s+[^/>]*))/?>(?!.*</\1>)#i';
+            $regexPattern = '#<(' . implode('|', self::SELF_CLOSING_TAGS) . ')((?>\s*[^/>]*))/?>(?!.*</\1>)#is';
         }
 
         $result = preg_replace($regexPattern, '<$1$2></$1>', $html);
@@ -58,7 +58,7 @@ public function afterSave($html)
         static $regexPattern = null;
 
         if (null === $regexPattern) {
-            $regexPattern = '#<(' . implode('|', self::SELF_CLOSING_TAGS) . ')((?>\s+[^>]*))>(?><\/\1>)#i';
+            $regexPattern = '#<(' . implode('|', self::SELF_CLOSING_TAGS) . ')((?>\s*[^>]*))>(?><\/\1>)#i';
         }
 
         $result = preg_replace($regexPattern, '<$1$2 />', $html);


Test patch:
diff --git a/tests/Dom/DocumentTest.php b/tests/Dom/DocumentTest.php
index a318daa45..316d02196 100644
--- a/tests/Dom/DocumentTest.php
+++ b/tests/Dom/DocumentTest.php
@@ -1429,8 +1429,8 @@ public function dataSVGSelfClosingElements()
                 .   '</svg>'
                 . '</body></html>',
                 '<!DOCTYPE html><html>' . $head . '<body>'
-                .   '<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">'
-                .   '    <path d="M 10,30 A 20,20 0,0,1 50,30 A 20,20 0,0,1 90,30 Q 90,60 50,90 Q 10,60 10,30 z"/>'
+                .   '<svg viewbox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">'
+                .   '    <path d="M 10,30 A 20,20 0,0,1 50,30 A 20,20 0,0,1 90,30 Q 90,60 50,90 Q 10,60 10,30 z" />'
                 .   '</svg>'
                 . '</body></html>',
             ],
@@ -1445,9 +1445,9 @@ public function dataSVGSelfClosingElements()
                 . '</svg>'
                 . '</body></html>',
                 '<!DOCTYPE html><html>' . $head . '<body>'
-                . '<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 24 24" fill="none">'
+                . '<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewbox="0 0 24 24" fill="none">'
                 . '    <path fill-rule="evenodd" clip-rule="evenodd" d="M11.99 2C6.47 2 2 6.48 2 12C2 17.52" fill="#1E1E1C" />'
-                . '    <mask id="mask0" maskUnits="userSpaceOnUse" x="2" y="2" width="20" height="20">'
+                . '    <mask id="mask0" maskunits="userSpaceOnUse" x="2" y="2" width="20" height="20">'
                 . '        <path fill-rule="evenodd" clip-rule="evenodd" d="M11.99 2C6.47 2 2 6.48 2 12C2 17.52" fill="white" />'
                 . '    </mask>'
                 . '    <g mask="url(#mask0)" />'
@@ -1464,13 +1464,29 @@ public function dataSVGSelfClosingElements()
                 . '</svg>'
                 . '</body></html>',
                 '<!DOCTYPE html><html>' . $head . '<body>'
-                . '<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">'
+                . '<svg viewbox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">'
                 . '    <g fill="white" stroke="green" stroke-width="5">'
                 . '        <circle cx="40" cy="40" r="25" />'
                 . '        <circle cx="60" cy="60" r="25" />'
                 . '    </g>'
                 . '</svg>'
                 . '</body></html>',
+            ],
+            'test g tag one path child and newlines' => [
+                '<!DOCTYPE html><html><head></head><body>' . PHP_EOL
+                . '<svg>' . PHP_EOL
+                . '  <g id="ok">' . PHP_EOL
+                . '    <path/>' . PHP_EOL
+                . '  </g>' . PHP_EOL
+                . '</svg>' . PHP_EOL
+                . '</body></html>',
+                '<!DOCTYPE html><html>' . $head . '<body>' . PHP_EOL
+                . '<svg>' . PHP_EOL
+                . '  <g id="ok">' . PHP_EOL
+                . '    <path />' . PHP_EOL
+                . '  </g>' . PHP_EOL
+                . '</svg>' . PHP_EOL
+                . '</body></html>',
             ]
         ];
     }
@@ -1485,7 +1501,7 @@ public function testSVGSelfClosingElements($source, $expected)
     {
         $document = Document::fromHtml($source);
         $output = $document->saveHTML();
-        $this->assertSimilarMarkup($expected, $output);
+        $this->assertEqualMarkup($expected, $output);
     }
 
     /**