Bug 134005

Summary:

Web Inspector: Add PrettyPrinter CSS tests

Product:

WebKit

Reporter:

Jonathan Wells <jonowells>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

burg, commit-queue, eric.carlson, glenn, graouts, jer.noble, joepeck, philipj, sergio, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

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.

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.