Bug 134005

Summary: Web Inspector: Add PrettyPrinter CSS tests
Product: WebKit Reporter: Jonathan Wells <jonowells>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Severity: Normal CC: burg, commit-queue, eric.carlson, glenn, graouts, jer.noble, joepeck, philipj, sergio, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
[PATCH] Proposed Fix none

Description Jonathan Wells 2014-06-17 16:51:50 PDT
There should be tests for CSS PrettyPrinting just as there are for JavaScript and HTML.

- WebKit prefixed
- Media queries
- Long lines
- Short lines
- Comments 
- Long set of rules
- Long rules
- Short rules
- Pseudo selectors
- url()

Comment 1 Joseph Pecoraro 2014-06-17 16:57:16 PDT
Glancing at the code for some of its edge cases:
- !important (should have space before)
- ":" // Space in "prop: value" but not in a selectors "a:link" or "div::after" or media queries "(max-device-width:480px)".
- multiple rules on one line => 2 newlines between rule declarations. (basically a bunch of minified CSS)

As long as we reduce multiple spaces to a single space we're doing pretty good because CSS enforces spaces between almost all tokens. Unlike JavaScript which has lots of operators that can be smashed together without spaces.
Comment 2 Radar WebKit Bug Importer 2014-06-17 16:59:01 PDT
Comment 3 Joseph Pecoraro 2014-10-03 15:48:22 PDT
Created attachment 239246 [details]
[PATCH] Proposed Fix
Comment 4 WebKit Commit Bot 2014-10-04 15:12:00 PDT
Comment on attachment 239246 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 239246

Committed r174323: <http://trac.webkit.org/changeset/174323>
Comment 5 WebKit Commit Bot 2014-10-04 15:12:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Brian Burg 2014-10-04 17:01:32 PDT
Why aren't these done as layout tests or tool tests? Will anyone remember to run the tests?
Comment 7 Joseph Pecoraro 2014-10-06 11:29:15 PDT
(In reply to comment #6)
> Why aren't these done as layout tests or tool tests? Will anyone remember to run the tests?

That is a good point. These could become layout tests. They only need to be run when we update CodeMirror, which is rare. I originally just added these as part of the tool because it was what I used for debugging and development, so it was easy and useful. We can move these.