Bug 134005 - Web Inspector: Add PrettyPrinter CSS tests
Summary: Web Inspector: Add PrettyPrinter CSS tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-17 16:51 PDT by Jonathan Wells
Modified: 2014-10-06 11:29 PDT (History)
11 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (16.29 KB, patch)
2014-10-03 15:48 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.

INITIAL IDEAS FOR TESTS TO ADD
- WebKit prefixed
- Media queries
- Long lines
- Short lines
- Comments 
- Long set of rules
- Long rules
- Short rules
- Pseudo selectors
- url()

Others?
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
<rdar://problem/17351848>
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.