1225192 - prettifying CSS preserves leading whitespace

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Tom Tromey :tromey]

When prettifying CSS, the code in css-logic preserves leading whitespace.
This can yield strange results like:

   div {
}

body {
}

Comment 1

[Tom Tromey :tromey]

Attachment: fix eslint complaints about css-logic.js

Cleaning up css-logic.js while I was in the area.

Comment 2

[Tom Tromey :tromey]

Attachment: remove leading whitespace before prettifying css

Remove leading whitespace before prettifying.

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8687991 [details] [diff] [review]
fix eslint complaints about css-logic.js

This fixes all the eslint nits I could fix.
(It still complains about domUtils and CSS not being defined,
but that's some other problem)

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8687991 [details] [diff] [review]
fix eslint complaints about css-logic.js

Review of attachment 8687991 [details] [diff] [review]:
-----------------------------------------------------------------

I'm going to be totally honest here, I did not review those 2000 lines in details :)
The changes seemed mechanical enough that I quickly glanced over at them.

What I did think about while reviewing though was that we probably are not using all of the code in this module anymore. Taking a quick look, I couldn't find a usage of findMatchedSelectors for example (which means that it could be removed along with compareTo and specificity). There might be some others too.
Also, the actors in styles.js are the only one to actually instantiate the CssLogic class, but I have a feeling that this class isn't nearly as useful as it used to. For instance, we keep on calling .reset() on the instance, which, I think, defeats the original purpose which was to cache a bunch of properties.

Anyway, I digress. So I filed bug 1225254.

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8687992 [details] [diff] [review]
remove leading whitespace before prettifying css

Review of attachment 8687992 [details] [diff] [review]:
-----------------------------------------------------------------

:) that was an easy one.

Comment 6

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d54cd280e9c5

Comment 7

[Tom Tromey :tromey]

Attachment: fix eslint complaints about css-logic.js

Rebased and added r= to commit.

Comment 8

[Tom Tromey :tromey]

Attachment: remove leading whitespace before prettifying css

try pointed out that some test cases would have to spuriously change,
because we're unconditionally rewriting the input.  This update
changes prettifyCSS to return the un-trimmed text for all "early"
exits.

Comment 9

[Tom Tromey :tromey]

Comment on attachment 8688615 [details] [diff] [review]
remove leading whitespace before prettifying css

Maybe it's silly to re-request review for this one.

Comment 10

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=515f3d01ea01

Comment 11

[Carsten Book [:Tomcat]]

this failed to apply:

renamed 1225192 -> Bug-1225192---fix-eslint-complaints-about-css-logi.patch
applying Bug-1225192---fix-eslint-complaints-about-css-logi.patch
patching file devtools/shared/styleinspector/css-logic.js
Hunk #2 FAILED at 75
1 out of 23 hunks FAILED -- saving rejects to file devtools/shared/styleinspector/css-logic.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh Bug-1225192---fix-eslint-complaints-about-css-logi.patch

Comment 12

[Tom Tromey :tromey]

I've rebased this but I think it's better to wait for bug 1216234 at this point,
as that one patches css-logic.js, which this patch massively changes.

Comment 13

[Tom Tromey :tromey]

Attachment: fix eslint complaints about css-logic.js

Rebased.

Comment 14

[Tom Tromey :tromey]

Attachment: remove leading whitespace before prettifying css

Rebased.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5c706bfd11cc
https://hg.mozilla.org/integration/fx-team/rev/872043399490

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c706bfd11cc
https://hg.mozilla.org/mozilla-central/rev/872043399490