New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614678 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit 29 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Formatting CSS breaks multiple class selectors

Reported by ngy...@gmail.com, May 25 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.6 Safari/537.36

Steps to reproduce the problem:
1. Open the attached test.htm which contain these source codes:

test.htm:
<link href="test.css" rel="stylesheet">
<div class="a b">test</div>

test.css:
.a.b {background:blue;}

2. Open Developer Tools, then the Sources section
3. On the left Sources section, choose test.css
4. Click the Format button (the two brackets icon)

What is the expected behavior?
The code gets prettified and the page doesn't break

What went wrong?
The CSS selector for `.a.b` is wrongly prettified and splits to `.a .b`, also breaking the page design/layout

Did this work before? Yes Chrome 50.0.2661.102

Chrome version: 51.0.2704.54  Channel: beta
OS Version: 7
Flash Version: 21.0.0.242

Broken on 52.0.2743.6 dev and 53.0.2748.0 canary too
 
Parallels Picture 12.png
108 KB View Download
test.htm
67 bytes View Download
test.css
23 bytes View Download
Components: -Platform>DevTools Platform>DevTools>Editing
Owner: lushnikov@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: -Pri-2 lusha-merge-tbd M-51 M-52 Pri-1
Status: Started (was: Assigned)
Thank you for the heads-up!

Started: https://codereview.chromium.org/2012953002/
Project Member

Comment 3 by bugdroid1@chromium.org, May 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aec0b6fa92b2f2bece418aede111a78b043a0655

commit aec0b6fa92b2f2bece418aede111a78b043a0655
Author: lushnikov <lushnikov@chromium.org>
Date: Fri May 27 00:39:14 2016

DevTools: fix css pretty-print.

The CSS pretty-printing was regressed back in the days when all formatters
were moved to a single content builder.

The content builder assumes that added tokens are "words" - and makes sure
those do not stick together. However, CSS formatter violates this contract.

This patch is a workaround which could be merged.

BUG= 614678 
R=dgozman

Review-Url: https://codereview.chromium.org/2012953002
Cr-Commit-Position: refs/heads/master@{#396332}

[rename] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1-expected.txt
[rename] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1.html
[add] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2-expected.txt
[add] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2.html
[add] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3-expected.txt
[add] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3.html
[modify] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/Source/devtools/front_end/formatter_worker/CSSFormatter.js
[modify] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/Source/devtools/front_end/formatter_worker/FormattedContentBuilder.js
[modify] https://crrev.com/aec0b6fa92b2f2bece418aede111a78b043a0655/third_party/WebKit/Source/devtools/front_end/formatter_worker/FormatterWorker.js

Labels: Merge-Request-52 Merge-Request-51
works fine on chrome 53.0.2750.0
It would be great to have this merged in M-51 and M-52. 
Krishna as per comment # 4, please approve the merges.

Comment 6 by gov...@chromium.org, May 27 2016

Labels: -Merge-Request-52 Merge-Approved-52
Approving merge to M52 branch 2743 based on comment #4.

Before we approve merge to M51, Could you please confirm whether this change is well baked in Canary and safe to merge?

Comment 7 by tin...@google.com, May 27 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.

Comment 8 by gov...@chromium.org, May 27 2016

Cc: sshruthi@chromium.org
+ sshruthi (M51 Desktop TPM), whether she is ok to take this merge in for M51 or not once we get reply for comment #6.
Project Member

Comment 9 by sheriffbot@chromium.org, May 31 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please have a the CL merged by EOD today(05/31), so it gets picked up for Beta Promotion scheduled on 06/02.
Cc: lushnikov@chromium.org
 Issue 615882  has been merged into this issue.
Labels: -Merge-Review-51 Merge-Approved-51
Merge approved for M51 (branch 2704)
Please your change to M51 branch 2704 by 4:00 PM PST today so we can take it for this week stable release. Thank you.
Project Member

Comment 14 by bugdroid1@chromium.org, May 31 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88787769105668050fd2958e860dd8835ce01357

commit 88787769105668050fd2958e860dd8835ce01357
Author: Andrey Lushnikov <lushnikov@chromium.org>
Date: Tue May 31 21:53:58 2016

DevTools: fix css pretty-print.

The CSS pretty-printing was regressed back in the days when all formatters
were moved to a single content builder.

The content builder assumes that added tokens are "words" - and makes sure
those do not stick together. However, CSS formatter violates this contract.

This patch is a workaround which could be merged.

BUG= 614678 
R=dgozman

Review-Url: https://codereview.chromium.org/2012953002
Cr-Commit-Position: refs/heads/master@{#396332}
(cherry picked from commit aec0b6fa92b2f2bece418aede111a78b043a0655)

Review URL: https://codereview.chromium.org/2028863002 .

Cr-Commit-Position: refs/branch-heads/2743@{#152}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[rename] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1-expected.txt
[rename] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1.html
[add] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2-expected.txt
[add] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2.html
[add] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3-expected.txt
[add] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3.html
[modify] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/Source/devtools/front_end/formatter_worker/CSSFormatter.js
[modify] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/Source/devtools/front_end/formatter_worker/FormattedContentBuilder.js
[modify] https://crrev.com/88787769105668050fd2958e860dd8835ce01357/third_party/WebKit/Source/devtools/front_end/formatter_worker/FormatterWorker.js

Labels: ReleaseBlock-Stable
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 1 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90c54581065ebdc1e8de454246915175664dbcde

commit 90c54581065ebdc1e8de454246915175664dbcde
Author: Andrey Lushnikov <lushnikov@chromium.org>
Date: Wed Jun 01 04:16:54 2016

DevTools: fix css pretty-print.

The CSS pretty-printing was regressed back in the days when all formatters
were moved to a single content builder.

The content builder assumes that added tokens are "words" - and makes sure
those do not stick together. However, CSS formatter violates this contract.

This patch is a workaround which could be merged.

BUG= 614678 
R=dgozman

Review-Url: https://codereview.chromium.org/2012953002
Cr-Commit-Position: refs/heads/master@{#396332}
(cherry picked from commit aec0b6fa92b2f2bece418aede111a78b043a0655)

Review URL: https://codereview.chromium.org/2023183002 .

Cr-Commit-Position: refs/branch-heads/2704@{#689}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[rename] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1-expected.txt
[rename] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-1.html
[add] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2-expected.txt
[add] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-2.html
[add] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3-expected.txt
[add] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/LayoutTests/inspector/sources/pretty-print-css-3.html
[modify] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/Source/devtools/front_end/formatter_worker/CSSFormatter.js
[modify] https://crrev.com/90c54581065ebdc1e8de454246915175664dbcde/third_party/WebKit/Source/devtools/front_end/formatter_worker/FormattedContentBuilder.js

Cc: dgozman@chromium.org
Cc: nyerramilli@chromium.org gov...@chromium.org
Labels: TE-Verified-M51 TE-Verified-51.0.2704.79
This is working fine in latest Canary #53.0.2754.0 and Stable # 51.0.2704.79 on Win 7.

attached screenshot for reference, adding 'TE-Verified' labels.
614678_Win.jpg
246 KB View Download
Please have a the CL merged to M52 branch-2743 by EOD today(06/01), so it gets picked up for Beta Promotion scheduled tomorrow.
Already merged in M52, thanks for the update.

lushnikov@ if there is nothing pending here, could you please tag as fixed.
This patch is now pushing out to stable channel in version 51.0.2704.79 for Desktop (Win,Mac & Linux).
Labels: -Merge-Approved-52
Per  Comment 14, this is already merged to M52 branch 2743. So removing "Merge-Approved-52" label.

Comment 24 by ajha@chromium.org, Jun 6 2016

lushnikov@: Please close this if there is no further work to be done on this & as the Fix is already merged to M-51.
Status: Fixed (was: Started)
[Bulk edit]

Our blockerbot script was offline; it was recently brought back online, and thus labeled many old issues (including this one) erroneously.  Removing Merge-TBD label since all milestones for this issue are already completed; no further work should be done.
Bad merge label applied, please ignore.
Components: Platform>DevTools>Authoring
Components: Platform>DevTools

Sign in to add a comment