Skip to content

Commit

Permalink
DevTools: Don't remove CSS semi colons that might be meaningful
Browse files Browse the repository at this point in the history
The CSS formatter tries to remove all duplicate semi colons. But if
there is a syntax error in the CSS, it might incorrectly remove
semicolons that would become meaningful.

Bug: 908243
Change-Id: I1757d432bfd8b4688e7179bb7937d85e8bea8da2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1381552
Reviewed-by: Erik Luo <luoe@chromium.org>
Commit-Queue: Joel Einbinder <einbinder@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642173}
  • Loading branch information
JoelEinbinder authored and Commit Bot committed Mar 19, 2019
1 parent 63563c2 commit c56204a
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 16 deletions.
28 changes: 12 additions & 16 deletions third_party/blink/renderer/devtools/front_end/sdk/CSSProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ SDK.CSSProperty = class {
* @param {boolean=} overwrite
* @return {!Promise.<boolean>}
*/
setText(propertyText, majorChange, overwrite) {
async setText(propertyText, majorChange, overwrite) {
if (!this.ownerStyle)
return Promise.reject(new Error('No ownerStyle for property'));

Expand All @@ -167,19 +167,9 @@ SDK.CSSProperty = class {
const text = new TextUtils.Text(this.ownerStyle.cssText || '');
const newStyleText = text.replaceRange(range, String.sprintf(';%s;', propertyText));

return self.runtime.extension(TextUtils.TokenizerFactory)
.instance()
.then(this._formatStyle.bind(this, newStyleText, indentation, endIndentation))
.then(setStyleText.bind(this));

/**
* @param {string} styleText
* @this {SDK.CSSProperty}
* @return {!Promise.<boolean>}
*/
function setStyleText(styleText) {
return this.ownerStyle.setText(styleText, majorChange);
}
const tokenizerFactory = await self.runtime.extension(TextUtils.TokenizerFactory).instance();
const styleText = SDK.CSSProperty._formatStyle(newStyleText, indentation, endIndentation, tokenizerFactory);
return this.ownerStyle.setText(styleText, majorChange);
}

/**
Expand All @@ -189,12 +179,13 @@ SDK.CSSProperty = class {
* @param {!TextUtils.TokenizerFactory} tokenizerFactory
* @return {string}
*/
_formatStyle(styleText, indentation, endIndentation, tokenizerFactory) {
static _formatStyle(styleText, indentation, endIndentation, tokenizerFactory) {
if (indentation)
indentation = '\n' + indentation;
let result = '';
let propertyText;
let insideProperty = false;
let needsSemi = false;
const tokenize = tokenizerFactory.createTokenizer('text/css');

tokenize('*{' + styleText + '}', processToken);
Expand All @@ -220,14 +211,19 @@ SDK.CSSProperty = class {
} else if (isPropertyStart) {
insideProperty = true;
propertyText = token;
} else if (token !== ';') {
} else if (token !== ';' || needsSemi) {
result += token;
if (token.trim() && !(tokenType && tokenType.includes('css-comment')))
needsSemi = token !== ';';
}
if (token === '{' && !tokenType)
needsSemi = false;
return;
}

if (token === '}' || token === ';') {
result = result.trimRight() + indentation + propertyText.trim() + ';';
needsSemi = false;
insideProperty = false;
if (token === '}')
result += '}';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
Tests that css properties are correctly formatted by the styles sidebar.

Raw CSS: color: red;
New CSS:
color: red;


Raw CSS: color: red;;;
New CSS:
color: red;


Raw CSS: color: red;;;color: blue
New CSS:
color: red;
color: blue;


Raw CSS: color: var(-);margin: 0;padding:0
New CSS:
color: var(-);margin: 0;padding:0


Raw CSS: color: red;/* a comment */;color: blue
New CSS:
color: red;/* a comment */
color: blue;


Raw CSS: :; color: red; color: blue
New CSS: :;
color: red;
color: blue;


Raw CSS: color: red;/* a comment;;; */ :; color: blue;
New CSS:
color: red;/* a comment;;; */ :;
color: blue;


Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

(async function() {
TestRunner.addResult(`Tests that css properties are correctly formatted by the styles sidebar.`);
await TestRunner.showPanel('elements');
const tokenizerFactory = await self.runtime.extension(TextUtils.TokenizerFactory).instance();

testText('color: red;');
testText('color: red;;;');
testText('color: red;;;color: blue');
testText('color: var(-);margin: 0;padding:0');
testText('color: red;/* a comment */;color: blue');
testText(`:; color: red; color: blue`);
testText('color: red;/* a comment;;; */ :; color: blue;')
TestRunner.completeTest();

function testText(cssText) {
TestRunner.addResult(`\nRaw CSS: ${cssText}`);
const newText = SDK.CSSProperty._formatStyle(cssText, ' ','', tokenizerFactory);
TestRunner.addResult(`New CSS: ${newText}`);
}

})();

0 comments on commit c56204a

Please sign in to comment.