Web Inspector: Fails to pretty-print a particular CSS file
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 May 2020 01:17:21 +0000 (01:17 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 May 2020 01:17:21 +0000 (01:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211930

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

* UserInterface/Workers/Formatter/CSSFormatter.js:
(CSSFormatter.prototype._format):
Keep a stack of special sequences (e.g. `"`, `'`, `/*`, etc.), only outputting the text in
between the start and end of the sequence when the stack is empty. Ignore all other special
sequences when in a comma sequence. Add proper checks for if the star/end is escaped.
Drive-by: minor refactor so that the arrow functions are created outside the loop.
LayoutTests:

* inspector/formatting/formatting-css.html:
* inspector/formatting/formatting-css-expected.txt:
* inspector/formatting/resources/css-tests/url.css: Added.
* inspector/formatting/resources/css-tests/url-expected.css: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261772 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/formatting/formatting-css-expected.txt
LayoutTests/inspector/formatting/formatting-css.html
LayoutTests/inspector/formatting/resources/css-tests/url-expected.css [new file with mode: 0644]
LayoutTests/inspector/formatting/resources/css-tests/url.css [new file with mode: 0644]
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Workers/Formatter/CSSFormatter.js

index 387fe65..fb91826 100644 (file)
@@ -1,3 +1,15 @@
+2020-05-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Fails to pretty-print a particular CSS file
+        https://bugs.webkit.org/show_bug.cgi?id=211930
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/formatting/formatting-css.html:
+        * inspector/formatting/formatting-css-expected.txt:
+        * inspector/formatting/resources/css-tests/url.css: Added.
+        * inspector/formatting/resources/css-tests/url-expected.css: Added.
+
 2020-05-15  Ryan Haddad  <ryanhaddad@apple.com>
 
         media/video-poster-set-after-playback.html is a flaky failure
index a941f5e..6489445 100644 (file)
@@ -9,5 +9,6 @@ PASS: gradient.css
 PASS: keyframes.css
 PASS: media-query.css
 PASS: selectors.css
+PASS: url.css
 PASS: wrapping.css
 
index f6526ad..e47659e 100644 (file)
@@ -15,6 +15,7 @@ function test()
         "resources/css-tests/keyframes.css",
         "resources/css-tests/media-query.css",
         "resources/css-tests/selectors.css",
+        "resources/css-tests/url.css",
         "resources/css-tests/wrapping.css",
     ]);
 
diff --git a/LayoutTests/inspector/formatting/resources/css-tests/url-expected.css b/LayoutTests/inspector/formatting/resources/css-tests/url-expected.css
new file mode 100644 (file)
index 0000000..7438ae1
--- /dev/null
@@ -0,0 +1,27 @@
+body {
+    background: url(basic);
+    background: url(with space);
+    background: url('  single  ');
+    background: url("  double  ");
+    background: url('  nested  "  double  "  single  ');
+    background: url("  nested  '  single  '  double  ");
+    background: url('  escaped  \'  single  \'  single  ');
+    background: url("  escaped  \"  double  \"  double  ");
+    background: url('  escaped  \\\'  single  \\\'  single  ');
+    background: url("  escaped  \\\"  double  \\\"  double  ");
+    background: url('  nested  /*  comment  */  single  ');
+    background: url("  nested  /*  comment  */  dobule  ");
+    background: url( /*  before  */ '  single  ' /*  after  */);
+    background: url( /*  before  */ "  double  " /*  after  */);
+}
+
+body {
+    background-size: cover;
+    background-repeat: no-repeat;
+    background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg clip-rule='evenodd' fill-rule='evenodd' stroke-linejoin='round' stroke-miterlimit='1.5' viewBox='0 0 200 200' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3CclipPath id='a'%3E%3Cpath d='M0 0h200v200H0z'/%3E%3C/clipPath%3E%3Cg clip-path='url(%23a)' transform='scale(0.5)'%3E%3Cimage height='200' transform='scale(2 2)' width='200' xlink:href='data:image/jpeg;base64,abcdef'/%3E%3C/g%3E%3C/svg%3E")
+}
+
+body {
+    background-image: linear-gradient(180deg, white, rgba(0, 0, 0, 0) 200px);
+    min-height: 200px
+}
diff --git a/LayoutTests/inspector/formatting/resources/css-tests/url.css b/LayoutTests/inspector/formatting/resources/css-tests/url.css
new file mode 100644 (file)
index 0000000..904a37f
--- /dev/null
@@ -0,0 +1,24 @@
+body {
+    background  :  url  (  basic  )  ;
+    background  :  url  (  with space  )  ;
+
+    background  :  url  (  '  single  '  )  ;
+    background  :  url  (  "  double  "  )  ;
+
+    background  :  url  (  '  nested  "  double  "  single  '  )  ;
+    background  :  url  (  "  nested  '  single  '  double  "  )  ;
+
+    background  :  url  (  '  escaped  \'  single  \'  single  '  )  ;
+    background  :  url  (  "  escaped  \"  double  \"  double  "  )  ;
+
+    background  :  url  (  '  escaped  \\\'  single  \\\'  single  '  )  ;
+    background  :  url  (  "  escaped  \\\"  double  \\\"  double  "  )  ;
+
+    background  :  url  (  '  nested  /*  comment  */  single  '  )  ;
+    background  :  url  (  "  nested  /*  comment  */  dobule  "  )  ;
+
+    background  :  url  (  /*  before  */  '  single  '  /*  after  */  )  ;
+    background  :  url  (  /*  before  */  "  double  "  /*  after  */  )  ;
+}
+
+body{background-size:cover;background-repeat:no-repeat;background-image:url("data:image/svg+xml;charset=utf-8,%3Csvg clip-rule='evenodd' fill-rule='evenodd' stroke-linejoin='round' stroke-miterlimit='1.5' viewBox='0 0 200 200' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3CclipPath id='a'%3E%3Cpath d='M0 0h200v200H0z'/%3E%3C/clipPath%3E%3Cg clip-path='url(%23a)' transform='scale(0.5)'%3E%3Cimage height='200' transform='scale(2 2)' width='200' xlink:href='data:image/jpeg;base64,abcdef'/%3E%3C/g%3E%3C/svg%3E")}body{background-image:linear-gradient(180deg,white,rgba(0,0,0,0) 200px);min-height:200px}
index 1028990..9b0af2d 100644 (file)
@@ -1,3 +1,17 @@
+2020-05-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Fails to pretty-print a particular CSS file
+        https://bugs.webkit.org/show_bug.cgi?id=211930
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Workers/Formatter/CSSFormatter.js:
+        (CSSFormatter.prototype._format):
+        Keep a stack of special sequences (e.g. `"`, `'`, `/*`, etc.), only outputting the text in
+        between the start and end of the sequence when the stack is empty. Ignore all other special
+        sequences when in a comma sequence. Add proper checks for if the star/end is escaped.
+        Drive-by: minor refactor so that the arrow functions are created outside the loop.
+
 2020-05-13  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: rename CSS.StyleSheetOrigin.Regular to CSS.StyleSheetOrigin.Author to match the spec
index d8766f8..282e47e 100644 (file)
@@ -91,167 +91,184 @@ CSSFormatter = class CSSFormatter
         const lastTokenWasOpenParenthesisRegExp = /\(\s*$/;
 
         let depth = 0;
+        let specialSequenceStack = [];
 
-        for (let i = 0; i < this._sourceText.length; ++i) {
-            let c = this._sourceText[i];
+        let index = 0;
+        let current = null;
 
-            let testCurrentLine = (regExp) => regExp.test(this._builder.currentLine);
+        let testCurrentLine = (regExp) => regExp.test(this._builder.currentLine);
 
-            let inComment = false;
-
-            let inSelector = () => {
-                let nextOpenBrace = this._sourceText.indexOf(`{`, i);
-                if (nextOpenBrace !== -1) {
-                    let nextQuote = Infinity;
-                    for (let quoteType of quoteTypes) {
-                        let quoteIndex = this._sourceText.indexOf(quoteType, i);
-                        if (quoteIndex !== -1 && quoteIndex < nextQuote)
-                            nextQuote = quoteIndex;
-                    }
-                    if (nextOpenBrace < nextQuote) {
-                        let nextSemicolon = this._sourceText.indexOf(`;`, i);
-                        if (nextSemicolon === -1)
-                            nextSemicolon = Infinity;
+        let inSelector = () => {
+            let nextOpenBrace = this._sourceText.indexOf(`{`, index);
+            if (nextOpenBrace !== -1) {
+                let nextQuote = Infinity;
+                for (let quoteType of quoteTypes) {
+                    let quoteIndex = this._sourceText.indexOf(quoteType, index);
+                    if (quoteIndex !== -1 && quoteIndex < nextQuote)
+                        nextQuote = quoteIndex;
+                }
+                if (nextOpenBrace < nextQuote) {
+                    let nextSemicolon = this._sourceText.indexOf(`;`, index);
+                    if (nextSemicolon === -1)
+                        nextSemicolon = Infinity;
 
-                        let nextNewline = this._sourceText.indexOf(`\n`, i);
-                        if (nextNewline === -1)
-                            nextNewline = Infinity;
+                    let nextNewline = this._sourceText.indexOf(`\n`, index);
+                    if (nextNewline === -1)
+                        nextNewline = Infinity;
 
-                        if (nextOpenBrace < Math.min(nextSemicolon, nextNewline))
-                            return true;
-                    }
+                    if (nextOpenBrace < Math.min(nextSemicolon, nextNewline))
+                        return true;
                 }
+            }
 
-                if (testCurrentLine(lineStartCouldBePropertyRegExp))
-                    return false;
+            if (testCurrentLine(lineStartCouldBePropertyRegExp))
+                return false;
 
-                return true;
-            };
+            return true;
+        };
 
-            let inProperty = () => {
-                if (!depth)
-                    return false;
-                return !testCurrentLine(inAtRuleRegExp) && !inSelector();
-            };
+        let inProperty = () => {
+            if (!depth)
+                return false;
+            return !testCurrentLine(inAtRuleRegExp) && !inSelector();
+        };
 
-            let formatBefore = () => {
-                if (this._builder.lastNewlineAppendWasMultiple && c === `}`)
-                    this._builder.removeLastNewline();
+        let formatBefore = () => {
+            if (this._builder.lastNewlineAppendWasMultiple && current === `}`)
+                this._builder.removeLastNewline();
 
-                if (dedentBefore.has(c))
-                    this._builder.dedent();
+            if (dedentBefore.has(current))
+                this._builder.dedent();
 
-                if (!this._builder.lastTokenWasNewline && newlineBefore.has(c))
-                    this._builder.appendNewline();
+            if (!this._builder.lastTokenWasNewline && newlineBefore.has(current))
+                this._builder.appendNewline();
 
-                if (!this._builder.lastTokenWasWhitespace && addSpaceBefore.has(c)) {
-                    let shouldAddSpaceBefore = () => {
-                        if (c === `(`) {
-                            if (testCurrentLine(inAtSupportsRuleRegExp))
+            if (!this._builder.lastTokenWasWhitespace && addSpaceBefore.has(current)) {
+                let shouldAddSpaceBefore = () => {
+                    if (current === `(`) {
+                        if (testCurrentLine(inAtSupportsRuleRegExp))
+                            return false;
+                        if (!testCurrentLine(inAtRuleRegExp))
+                            return false;
+                    }
+                    return true;
+                };
+                if (shouldAddSpaceBefore())
+                    this._builder.appendSpace();
+            }
+
+            while (this._builder.lastTokenWasWhitespace && removeSpaceBefore.has(current)) {
+                let shouldRemoveSpaceBefore = () => {
+                    if (current === `:`) {
+                        if (!testCurrentLine(this._builder.currentLine.includes(`(`) ? inAtRuleRegExp : inAtRuleBeforeParenthesisRegExp)) {
+                            if (!inProperty())
                                 return false;
-                            if (!testCurrentLine(inAtRuleRegExp))
+                        }
+                    }
+                    if (current === `(`) {
+                        if (!testCurrentLine(lastTokenWasOpenParenthesisRegExp)) {
+                            if (testCurrentLine(inAtRuleRegExp) && !testCurrentLine(inAtRuleAfterParenthesisRegExp))
                                 return false;
                         }
-                        return true;
-                    };
-                    if (shouldAddSpaceBefore())
-                        this._builder.appendSpace();
-                }
+                    }
+                    return true;
+                };
+                if (!shouldRemoveSpaceBefore())
+                    break;
+                this._builder.removeLastWhitespace();
+            }
+        };
 
-                while (this._builder.lastTokenWasWhitespace && removeSpaceBefore.has(c)) {
-                    let shouldRemoveSpaceBefore = () => {
-                        if (c === `:`) {
-                            if (!testCurrentLine(this._builder.currentLine.includes(`(`) ? inAtRuleRegExp : inAtRuleBeforeParenthesisRegExp)) {
+        let formatAfter = () => {
+            while (this._builder.lastTokenWasWhitespace && removeSpaceAfter.has(current)) {
+                let shouldRemoveSpaceAfter = () => {
+                    if (current === `(`) {
+                        if (!testCurrentLine(lastTokenWasOpenParenthesisRegExp)) {
+                            if (!testCurrentLine(inAtRuleRegExp)) {
                                 if (!inProperty())
                                     return false;
                             }
                         }
-                        if (c === `(`) {
-                            if (!testCurrentLine(lastTokenWasOpenParenthesisRegExp)) {
-                                if (testCurrentLine(inAtRuleRegExp) && !testCurrentLine(inAtRuleAfterParenthesisRegExp))
-                                    return false;
-                            }
-                        }
-                        return true;
-                    };
-                    if (!shouldRemoveSpaceBefore())
-                        break;
-                    this._builder.removeLastWhitespace();
-                }
-            };
-
-            let formatAfter = () => {
-                while (this._builder.lastTokenWasWhitespace && removeSpaceAfter.has(c)) {
-                    let shouldRemoveSpaceAfter = () => {
-                        if (c === `(`) {
-                            if (!testCurrentLine(lastTokenWasOpenParenthesisRegExp)) {
-                                if (!testCurrentLine(inAtRuleRegExp)) {
-                                    if (!inProperty())
-                                        return false;
-                                }
-                            }
-                        }
-                        return true;
-                    };
-                    if (!shouldRemoveSpaceAfter())
-                        break;
-                    this._builder.removeLastWhitespace();
-                }
+                    }
+                    return true;
+                };
+                if (!shouldRemoveSpaceAfter())
+                    break;
+                this._builder.removeLastWhitespace();
+            }
 
-                if (!this._builder.lastTokenWasWhitespace && addSpaceAfter.has(c)) {
-                    let shouldAddSpaceAfter = () => {
-                        if (c === `:`) {
-                            if (!testCurrentLine(this._builder.currentLine.includes(`(`) ? inAtRuleRegExp : inAtRuleBeforeParenthesisRegExp)) {
-                                if (!inProperty())
-                                    return false;
-                            }
+            if (!this._builder.lastTokenWasWhitespace && addSpaceAfter.has(current)) {
+                let shouldAddSpaceAfter = () => {
+                    if (current === `:`) {
+                        if (!testCurrentLine(this._builder.currentLine.includes(`(`) ? inAtRuleRegExp : inAtRuleBeforeParenthesisRegExp)) {
+                            if (!inProperty())
+                                return false;
                         }
-                        if (c === `)`) {
-                            if (!testCurrentLine(inAtRuleRegExp)) {
-                                if (!inProperty())
-                                    return false;
-                            }
+                    }
+                    if (current === `)`) {
+                        if (!testCurrentLine(inAtRuleRegExp)) {
+                            if (!inProperty())
+                                return false;
                         }
-                        return true;
-                    };
-                    if (shouldAddSpaceAfter())
-                        this._builder.appendSpace();
-                }
+                    }
+                    return true;
+                };
+                if (shouldAddSpaceAfter())
+                    this._builder.appendSpace();
+            }
+
+            if (indentAfter.has(current))
+                this._builder.indent();
 
-                if (indentAfter.has(c))
-                    this._builder.indent();
+            if (newlineAfter.has(current)) {
+                if (current === `}`)
+                    this._builder.appendMultipleNewlines(2);
+                else
+                    this._builder.appendNewline();
+            }
+        };
+
+        for (; index < this._sourceText.length; ++index) {
+            current = this._sourceText[index];
+
+            let possibleSpecialSequence = null;
+            if (quoteTypes.has(current))
+                possibleSpecialSequence = {type: "quote", startIndex: index, endString: current};
+            else if (current === `/` && this._sourceText[index + 1] === `*`)
+                possibleSpecialSequence = {type: "comment", startIndex: index, endString: `*/`};
+            else if (current === `u` && this._sourceText[index + 1] === `r` && this._sourceText[index + 2] === `l` && this._sourceText[index + 3] === `(`)
+                possibleSpecialSequence = {type: "url", startIndex: index, endString: `)`};
+
+            if (possibleSpecialSequence || specialSequenceStack.length) {
+                let currentSpecialSequence = specialSequenceStack.lastValue;
+
+                if (currentSpecialSequence?.type !== "comment") {
+                    if (possibleSpecialSequence?.type !== "comment") {
+                        let escapeCount = 0;
+                        while (this._sourceText[index - 1 - escapeCount] === "\\")
+                            ++escapeCount;
+                        if (escapeCount % 2)
+                            continue;
+                    }
 
-                if (newlineAfter.has(c)) {
-                    if (c === `}`)
-                        this._builder.appendMultipleNewlines(2);
-                    else
-                        this._builder.appendNewline();
+                    if (possibleSpecialSequence && (!currentSpecialSequence || currentSpecialSequence.type !== possibleSpecialSequence.type || currentSpecialSequence.endString !== possibleSpecialSequence.endString)) {
+                        specialSequenceStack.push(possibleSpecialSequence);
+                        continue;
+                    }
                 }
-            };
-
-            let specialSequenceEnd = null;
-            if (quoteTypes.has(c))
-                specialSequenceEnd = c;
-            else if (c === `/` && this._sourceText[i + 1] === `*`) {
-                inComment = true;
-                specialSequenceEnd = `*/`;
-            } else if (c === `u` && this._sourceText[i + 1] === `r` && this._sourceText[i + 2] === `l` && this._sourceText[i + 3] === `(`)
-                specialSequenceEnd = `)`;
-
-            if (specialSequenceEnd) {
-                let startIndex = i;
-                let endIndex = i;
-                do {
-                    endIndex = this._sourceText.indexOf(specialSequenceEnd, endIndex + specialSequenceEnd.length);
-                } while (endIndex !== -1 && !inComment && this._sourceText[endIndex - 1] === `\\`);
-
-                if (endIndex === -1)
-                    endIndex = this._sourceText.length;
-                endIndex += specialSequenceEnd.length;
-
-                let specialSequenceText = this._sourceText.substring(startIndex, endIndex);
-
-                let lastSourceNewlineWasMultiple = this._sourceText[startIndex - 1] === `\n` && this._sourceText[startIndex - 2] === `\n`;
+
+                if (Array.from(currentSpecialSequence.endString).some((item, i) => index + i < this._sourceText.length && this._sourceText[index - currentSpecialSequence.endString.length + i + 1] !== item))
+                    continue;
+
+                let inComment = specialSequenceStack.some((item) => item.type === "comment");
+
+                specialSequenceStack.pop();
+                if (specialSequenceStack.length)
+                    continue;
+
+                let specialSequenceText = this._sourceText.substring(currentSpecialSequence.startIndex, index + 1);
+
+                let lastSourceNewlineWasMultiple = this._sourceText[currentSpecialSequence.startIndex - 1] === `\n` && this._sourceText[currentSpecialSequence.startIndex - 2] === `\n`;
                 let lastAppendNewlineWasMultiple = this._builder.lastNewlineAppendWasMultiple;
 
                 let commentOnOwnLine = false;
@@ -264,7 +281,7 @@ CSSFormatter = class CSSFormatter
                     }
 
                     if (commentOnOwnLine) {
-                        if (startIndex > 0 && !this._builder.indented)
+                        if (currentSpecialSequence.startIndex > 0 && !this._builder.indented)
                             this._builder.appendNewline();
                     } else if (this._builder.currentLine.length && !this._builder.lastTokenWasWhitespace)
                         this._builder.appendSpace();
@@ -273,10 +290,7 @@ CSSFormatter = class CSSFormatter
                         this._builder.appendNewline(true);
                 }
 
-                this._builder.appendStringWithPossibleNewlines(specialSequenceText, startIndex);
-
-                i = endIndex;
-                c = this._sourceText[i];
+                this._builder.appendStringWithPossibleNewlines(specialSequenceText, currentSpecialSequence.startIndex);
 
                 if (inComment) {
                     if (commentOnOwnLine) {
@@ -284,7 +298,7 @@ CSSFormatter = class CSSFormatter
                             this._builder.appendMultipleNewlines(2);
                         else
                             this._builder.appendNewline();
-                    } else if (!/\s/.test(c)) {
+                    } else if (!/\s/.test(current)) {
                         if (!testCurrentLine(inAtRuleRegExp) && !inSelector() && !inProperty())
                             this._builder.appendNewline();
                         else
@@ -292,15 +306,12 @@ CSSFormatter = class CSSFormatter
                     }
                 }
 
-                --i; // Account for the iteration of the for loop.
-                c = this._sourceText[i];
-
                 formatAfter();
                 continue;
             }
 
-            if (/\s/.test(c)) {
-                if (c === `\n`) {
+            if (/\s/.test(current)) {
+                if (current === `\n`) {
                     if (!this._builder.lastTokenWasNewline) {
                         while (this._builder.lastTokenWasWhitespace)
                             this._builder.removeLastWhitespace();
@@ -314,16 +325,21 @@ CSSFormatter = class CSSFormatter
                 continue;
             }
 
-            if (c === `{`)
+            if (current === `{`)
                 ++depth;
-            else if (c === `}`)
+            else if (current === `}`)
                 --depth;
 
             formatBefore();
-            this._builder.appendToken(c, i);
+            this._builder.appendToken(current, index);
             formatAfter();
         }
 
+        if (specialSequenceStack.length) {
+            let firstSpecialSequence = specialSequenceStack[0];
+            this._builder.appendStringWithPossibleNewlines(this._sourceText.substring(firstSpecialSequence.startIndex), firstSpecialSequence.startIndex);
+        }
+
         this._builder.finish();
     }
 };