[BUGFIX] Do not escape characters that do not need escaping in CSS string (#1444)

Author: 8ctopus

CHANGELOG.md

@@ -26,6 +26,7 @@ Please also have a look at our
 
 ### Fixed
 
+- Do not escape characters that do not need escaping in CSS string (#1444)
 - Reject selector comprising only whitespace (#1433)
 - Improve recovery parsing when a rogue `}` is encountered (#1425, #1426)
 - Parse comment(s) immediately preceding a selector (#1421, #1424)

src/Value/CSSString.php

@@ -93,7 +93,7 @@ public function getString(): string
      */
     public function render(OutputFormat $outputFormat): string
     {
-        $string = \addslashes($this->string);
+        $string = $this->escape($this->string, $outputFormat->getStringQuotingType());
         $string = \str_replace("\n", '\\A', $string);
         return $outputFormat->getStringQuotingType() . $string . $outputFormat->getStringQuotingType();
     }
@@ -111,4 +111,15 @@ public function getArrayRepresentation(): array
             'contents' => $this->string,
         ];
     }
+
+    /**
+     * @param "'"|'"' $quote
+     */
+    private function escape(string $string, string $quote): string
+    {
+        $charactersToEscape = '\\';
+        $charactersToEscape .= ($quote === '"' ? '"' : "'");
+
+        return \addcslashes($string, $charactersToEscape);
+    }
 }

tests/ParserTest.php

@@ -206,10 +206,10 @@ public function unicodeParsing(): void
                 self::assertSame('"\\"\\""', $firstContentRuleAsString);
             }
             if ($selector === '.test-9') {
-                self::assertSame('"\\"\\\'"', $firstContentRuleAsString);
+                self::assertSame('"\\"\'"', $firstContentRuleAsString);
             }
             if ($selector === '.test-10') {
-                self::assertSame('"\\\'\\\\"', $firstContentRuleAsString);
+                self::assertSame('"\'\\\\"', $firstContentRuleAsString);
             }
             if ($selector === '.test-11') {
                 self::assertSame('"test"', $firstContentRuleAsString);

tests/Unit/Value/CSSStringTest.php

@@ -5,6 +5,7 @@
 namespace Sabberworm\CSS\Tests\Unit\Value;
 
 use PHPUnit\Framework\TestCase;
+use Sabberworm\CSS\OutputFormat;
 use Sabberworm\CSS\Value\CSSString;
 use Sabberworm\CSS\Value\PrimitiveValue;
 use Sabberworm\CSS\Value\Value;
@@ -107,4 +108,42 @@ public function getArrayRepresentationIncludesContents(): void
         self::assertArrayHasKey('contents', $result);
         self::assertSame($contents, $result['contents']);
     }
+
+    /**
+     * @test
+     */
+    public function doesNotEscapeSingleQuotesThatDoNotNeedToBeEscaped(): void
+    {
+        $input = "data:image/svg+xml,%3Csvg width='24' height='24' viewBox='0 0 24 24' fill='none'" .
+            " xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd'" .
+            " d='M14.3145 2 3V11.9987H17.5687L18 8.20761H14.3145L14.32 6.31012C14.32 5.32134 14.4207" .
+            ' 4.79153 15.9426 4.79153H17.977V1H14.7223C10.8129 1 9.43687 2.83909 9.43687 ' .
+            " 5.93187V8.20804H7V11.9991H9.43687V23H14.3145Z' fill='black'/%3E%3C/svg%3E%0A";
+
+        $outputFormat = OutputFormat::createPretty();
+
+        self::assertSame("\"{$input}\"", (new CSSString($input))->render($outputFormat));
+
+        $outputFormat->setStringQuotingType("'");
+
+        $expected = str_replace("'", "\\'", $input);
+
+        self::assertSame("'{$expected}'", (new CSSString($input))->render($outputFormat));
+    }
+
+    /**
+     * @test
+     */
+    public function doesNotEscapeDoubleQuotesThatDoNotNeedToBeEscaped(): void
+    {
+        $input = '"Hello World"';
+
+        $outputFormat = OutputFormat::createPretty();
+
+        self::assertSame('"\\"Hello World\\""', (new CSSString($input))->render($outputFormat));
+
+        $outputFormat->setStringQuotingType("'");
+
+        self::assertSame("'{$input}'", (new CSSString($input))->render($outputFormat));
+    }
 }

tests/fixtures/unicode.css

@@ -5,7 +5,7 @@
 .test-6 { content: "\00A5" } /* Same as "¥" */
 .test-7 { content: '\a' } /* Same as "\A" (Newline) */
 .test-8 { content: "\"\22" } /* Same as "\"\"" */
-.test-9 { content: "\"\27" } /* Same as ""\"\'"" */
+.test-9 { content: "\"\27" } /* Same as "\"'" */
 .test-10 { content: "\'\\" } /* Same as "'\" */
 .test-11 { content: "\test" } /* Same as "test" */