Closed Bug 711401 Opened 8 years ago Closed 6 years ago

Many style sheets are not prettified / deobfuscated automatically

Categories

(DevTools :: Style Editor, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 30

People

(Reporter: dangoor, Assigned: harth)

References

Details

Attachments

(1 file)

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)
Duplicate of this bug: 783808
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Duplicate of this bug: 905180
Assignee: jryans → nobody
Status: ASSIGNED → NEW
Summary: Many style sheets are not prettified automatically → Many style sheets are not prettified / deobfuscated automatically
OS: Mac OS X → All
Hardware: x86 → All
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
Assignee: nobody → fayearthur
Attachment #8390842 - Flags: review?(jwalker)
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.
Status: NEW → ASSIGNED
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?
Attachment #8390842 - Flags: review?(jwalker) → review+
(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.
Keywords: checkin-needed
(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.
https://hg.mozilla.org/mozilla-central/rev/30f5d8030219
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
Duplicate of this bug: 945137
QA Whiteboard: [qa-]
Depends on: 1031351
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.