Bug 146671 - Web Inspector: Unnecessary space added after -webkit- prefixed property values
Summary: Web Inspector: Unnecessary space added after -webkit- prefixed property values
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: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-06 23:01 PDT by Devin Rousso
Modified: 2015-07-07 14:22 PDT (History)
8 users (show)

See Also:


Attachments
Actual (20.63 KB, image/png)
2015-07-06 23:01 PDT, Devin Rousso
no flags Details
Patch (1.56 KB, patch)
2015-07-06 23:07 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Video of Current Functionality (741.24 KB, video/quicktime)
2015-07-07 00:37 PDT, Devin Rousso
no flags Details
Patch (4.10 KB, patch)
2015-07-07 01:17 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (4.12 KB, patch)
2015-07-07 13:15 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (4.07 KB, patch)
2015-07-07 13:20 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2015-07-06 23:01:27 PDT
Created attachment 256273 [details]
Actual

* STEPS TO REPRODUCE
1. Add the following CSS to any rule in the sidebar:

width: -webkit-calc(1px);

2. After clicking outside of that rule (to remove focus), the CSS will be reformatted as:

width: -webkit- calc(1px);a
Comment 1 Radar WebKit Bug Importer 2015-07-06 23:02:00 PDT
<rdar://problem/21698881>
Comment 2 Devin Rousso 2015-07-06 23:07:47 PDT
Created attachment 256274 [details]
Patch
Comment 3 Joseph Pecoraro 2015-07-06 23:15:30 PDT
Comment on attachment 256274 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256274&action=review

r-, we should add a test for this.
Source/WebInspectorUI/Tools/PrettyPrinting/css-rule-tests

Why don't I see it happen on this when I load the test case?
Source/WebInspectorUI/Tools/PrettyPrinting/index.html?mode=css-rule&content=width%3A%20-webkit-calc(1px)%3B

> Source/WebInspectorUI/ChangeLog:9
> +        * UserInterface/Views/CodeMirrorFormatters.js: Now only adds a space if both the current and previous tokens are a property, value,
> +        or atom.

Gross formatting! Wrap a little earlier so this is more comfortable to read.
Comment 4 Devin Rousso 2015-07-07 00:37:56 PDT
Created attachment 256287 [details]
Video of Current Functionality

This is what is currently happening on my release build.
Comment 5 Devin Rousso 2015-07-07 01:17:51 PDT
Created attachment 256289 [details]
Patch
Comment 6 Devin Rousso 2015-07-07 13:15:38 PDT
Created attachment 256316 [details]
Patch
Comment 7 Joseph Pecoraro 2015-07-07 13:17:50 PDT
Comment on attachment 256316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256316&action=review

r=me, thanks for addressing all my comments!

> Source/WebInspectorUI/Tools/PrettyPrinting/index.html:54
> +    // Here, the "calc" keyword is added to allow  the do-not-add-whitespace-before-prefixed-property-value

Nit: Double space. Also don't call out a particular test by name, that is the kind of comment that can go stale in the future if that test gets removed or renamed. Just mention its to behave more like the frontend.
Comment 8 Devin Rousso 2015-07-07 13:20:20 PDT
Created attachment 256317 [details]
Patch
Comment 9 WebKit Commit Bot 2015-07-07 14:22:41 PDT
Comment on attachment 256317 [details]
Patch

Clearing flags on attachment: 256317

Committed r186477: <http://trac.webkit.org/changeset/186477>
Comment 10 WebKit Commit Bot 2015-07-07 14:22:46 PDT
All reviewed patches have been landed.  Closing bug.