Bug 134704

Summary: Web Inspector: CSS style declaration editor should ignore empty lines when determining prefix whitespace
Product: WebKit Reporter: Jonathan Wells <jonowells>
Component: Web InspectorAssignee: Jonathan Wells <jonowells>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Attempted fix. none

Description Jonathan Wells 2014-07-07 16:58:37 PDT
The CSS style editor looks for the line with the shortest prefixed whitespace and strips that from every line, the idea being that the baseline indentation is ignored and the rules look better. Indentation relative to that baseline is honored.

The change made in https://bugs.webkit.org/show_bug.cgi?id=133515 properly handles lines with zero-length prefixed whitespace lines, but doesn't consider completely empty lines in cases such as:

div {
    font-weight: bold;

    color: red;
    display: inline;
}

The empty line here lacks any spaces or tabs. In this case the empty line should be ignored as it is likely the author is eliminating all trailing whitespace from the document. This is so common that many inspected style declarations appear indented when they should not.
Comment 1 Radar WebKit Bug Importer 2014-07-07 16:58:45 PDT
<rdar://problem/17583210>
Comment 2 Jonathan Wells 2014-07-07 17:07:34 PDT
Created attachment 234527 [details]
[PATCH] Attempted fix.
Comment 3 Joseph Pecoraro 2014-07-07 18:06:28 PDT
Comment on attachment 234527 [details]
[PATCH] Attempted fix.

Awesome. r=me
Comment 4 WebKit Commit Bot 2014-07-07 18:39:03 PDT
Comment on attachment 234527 [details]
[PATCH] Attempted fix.

Clearing flags on attachment: 234527

Committed r170868: <http://trac.webkit.org/changeset/170868>
Comment 5 WebKit Commit Bot 2014-07-07 18:39:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Timothy Hatcher 2014-07-08 12:12:18 PDT
Comment on attachment 234527 [details]
[PATCH] Attempted fix.

Nice fix! We will likely add the space o the empty line in _formattedContent before committing the change. We likely want to only do that for lines with content to prevent unneeded whitespace. Not critical though.