711401 - Many style sheets are not prettified / deobfuscated automatically

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Kevin Dangoor]

I thought I had filed this previously, but I can't find the bug now.

The Style Editor prettifies style sheets automatically. However, the heuristic it uses to determine whether a style sheet *needs* to be prettified is too conservative. It works for github.com (which was Cedric's test case), but it doesn't work in many other places I've looked.

gmail has a newline at the beginning.
cnn.com has 23 lines
WordPress admin interface has 2 lines (and a blank line at the end)

Comment 4

[Heather Arthur [:harth]]

Attachment: Pretty print even with spaces and newlines at the end of the file

It turns out that we bail on pretty printing if there's any whitespace after a "{", ";", or "{".

This patch will allow any amount of whitespace at the end of a file (it already allows any amount at the beginning), so http://github.com works now. It also allows spaces as I'm seeing files with spaces after rules not newlines, that aren't being pretty printed (http://gmail.com).

It reenables the pretty printing test that was disabled in bug 942473. I'm going to run try to see what happens there: https://tbpl.mozilla.org/?tree=Try&rev=8a17c95eeb8d

Comment 5

[Girish Sharma [:Optimizer]]

One thought - Like debugger, we might also want to provide an option to disable this feature. One of the most useful feature of Style Editor is that you can modify the CSS file, see the live changes, get to a satisfactory result and then *save it back to disk* replacing the original file that was loaded earlier. I might not want to mess up with already existing formatting over the whole file for a couple of line changes.

Comment 6

[Joe Walker [:jwalker]]

Comment on attachment 8390842 [details] [diff] [review]
Pretty print even with spaces and newlines at the end of the file

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

::: browser/devtools/styleeditor/test/browser.ini
@@ +13,5 @@
>    media-small.css
>    media.html
>    minified.html
>    nostyle.html
> +  pretty.css

Is this actually used? Maybe you inlined it and forgot to take it out?

@@ -45,5 @@
>  [browser_styleeditor_new.js]
>  [browser_styleeditor_nostyle.js]
>  [browser_styleeditor_pretty.js]
> -# Disabled because of intermittent failures - See Bug 942473
> -skip-if = true

Assuming this was fixed by your recent intermittent fix, right?

Comment 7

[Heather Arthur [:harth]]

(In reply to Joe Walker [:jwalker] from comment #6)
> Comment on attachment 8390842 [details] [diff] [review]
> Pretty print even with spaces and newlines at the end of the file
> 
> Review of attachment 8390842 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/styleeditor/test/browser.ini
> @@ +13,5 @@
> >    media-small.css
> >    media.html
> >    minified.html
> >    nostyle.html
> > +  pretty.css
> 
> Is this actually used? Maybe you inlined it and forgot to take it out?

So weird, the patch has it, but the Review doesn't show it. I add a <link> to the minified.html.

> 
> @@ -45,5 @@
> >  [browser_styleeditor_new.js]
> >  [browser_styleeditor_nostyle.js]
> >  [browser_styleeditor_pretty.js]
> > -# Disabled because of intermittent failures - See Bug 942473
> > -skip-if = true
> 
> Assuming this was fixed by your recent intermittent fix, right?

CSS source maps landed after that, which changed the code a good bit. Unfortunately the logs in bug 942473 have expired and I can't reproduce the problem locally or on try. So I'm hoping it's fixed now. If not then we can take another look.

Comment 8

[Heather Arthur [:harth]]

https://hg.mozilla.org/integration/fx-team/rev/30f5d8030219

Comment 9

[Joe Walker [:jwalker]]

(In reply to Heather Arthur [:harth] from comment #7)
> > Assuming this was fixed by your recent intermittent fix, right?
> 
> CSS source maps landed after that, which changed the code a good bit.
> Unfortunately the logs in bug 942473 have expired and I can't reproduce the
> problem locally or on try. So I'm hoping it's fixed now. If not then we can
> take another look.

Also that's intermittent to the tune of caused-by-proton-decay.
Like, happened only once. Ever.

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/30f5d8030219