Bug 198806 – Web Inspector: REGRESSION(r246178): extra spaces added in at-rules when formatting CSS

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

.pagination .page-numbers:not(.current, .dots):hover {
  --- becomes ---
.pagination .page-numbers:not(.current, .dots) :hover {

[Devin Rousso]

*** Bug 198807 has been marked as a duplicate of this bug. ***

[Devin Rousso]

Created attachment 372026 [details]
Patch

This could use a better explanation, but I'm too tired right now, so I'll do it later :(

[Joseph Pecoraro]

Comment on attachment 372026 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372026&action=review

rs=me

> Source/WebInspectorUI/UserInterface/Workers/Formatter/CSSFormatter.js:85
> +                let re = new RegExp(`^\\s*@[a-zA-Z][-a-zA-Z]+${suffix}`);

Seems like we may be building these regexes over and over again, which could be bad for performance.

> Source/WebInspectorUI/UserInterface/Workers/Formatter/CSSFormatter.js:86
> +                return re.test(this._builder.currentLine);

What if this isn't on the same line?

    @media
    (...)
    { ... }

> Source/WebInspectorUI/UserInterface/Workers/Formatter/CSSFormatter.js:89
> +            let inSelector = (suffix = "") => {

`suffix` is unused.

[Devin Rousso]

Comment on attachment 372026 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372026&action=review

>> Source/WebInspectorUI/UserInterface/Workers/Formatter/CSSFormatter.js:86
>> +                return re.test(this._builder.currentLine);
> 
> What if this isn't on the same line?
> 
>     @media
>     (...)
>     { ... }

`currentLine` refers to the last line of the `FormatterContentBuilder`, so basically everything up to the character `c` after the last `appendNewline`.

[Devin Rousso]

Created attachment 372092 [details]
Patch

[EWS Watchlist]

Comment on attachment 372092 [details]
Patch

Attachment 372092 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12470795

New failing tests:
imported/blink/fast/canvas/canvas-state-persistence-no-dirty.html
js/slow-stress/array-prototype-filter.html
css3/filters/blur-various-radii.html

[EWS Watchlist]

Created attachment 372106 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[WebKit Commit Bot]

Comment on attachment 372092 [details]
Patch

Clearing flags on attachment: 372092

Committed r246430: <https://trac.webkit.org/changeset/246430>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51737451>