Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Web Inspector: CSS autocomplete: property usage counts should ignore variables #2124

Merged
merged 1 commit into from Jul 6, 2022
Merged

Web Inspector: CSS autocomplete: property usage counts should ignore variables #2124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions LayoutTests/inspector/css/css-property-expected.txt
View file
Open in desktop
Expand Up @@ -8,6 +8,8 @@ PASS: "background-repeat" should have at least 1 count.
PASS: "background-repeat-x" should not be counted.
PASS: "background-repeat-y" should not be counted.
PASS: "background-repeat-invalid" should not be counted.
PASS: "--foo" should not be counted.
PASS: "--bar" should not be counted.

Sorted by usage:
[
Expand Down
4 changes: 4 additions & 0 deletions LayoutTests/inspector/css/css-property.html
View file
Open in desktop
Expand Up @@ -17,6 +17,8 @@
InspectorTest.expectThat(!("background-repeat-x" in WI.CSSProperty._cachedNameCounts), `"background-repeat-x" should not be counted.`);
InspectorTest.expectThat(!("background-repeat-y" in WI.CSSProperty._cachedNameCounts), `"background-repeat-y" should not be counted.`);
InspectorTest.expectThat(!("background-repeat-invalid" in WI.CSSProperty._cachedNameCounts), `"background-repeat-invalid" should not be counted.`);
InspectorTest.expectThat(!("--foo" in WI.CSSProperty._cachedNameCounts), `"--foo" should not be counted.`);
InspectorTest.expectThat(!("--bar" in WI.CSSProperty._cachedNameCounts), `"--bar" should not be counted.`);

InspectorTest.newline();

Expand Down Expand Up @@ -300,6 +302,8 @@
background-repeat: repeat;
background-repeat-x: repeat;
background-repeat-invalid: repeat;
--foo: red;
--bar: blue;
/* background-color: black; */
/* Not a CSS property */
/* foo:bar; foo:baz; */
Expand Down
15 changes: 11 additions & 4 deletions Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
View file
Open in desktop
Expand Up @@ -43,13 +43,17 @@ WI.CSSProperty = class CSSProperty extends WI.Object

// Static

static isVariable(name)
{
return name.startsWith("--") && name.length > 2;
}

static isInheritedPropertyName(name)
{
console.assert(typeof name === "string");
if (WI.CSSKeywordCompletions.InheritedProperties.has(name))
return true;
// Check if the name is a CSS variable.
return name.startsWith("--");
return WI.CSSProperty.isVariable(name);
}

// FIXME: <https://webkit.org/b/226647> This naively collects variable-like names used in values. It should be hardened.
Expand Down Expand Up @@ -215,7 +219,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
this._anonymous = anonymous;
this._inherited = WI.CSSProperty.isInheritedPropertyName(name);
this._valid = valid;
this._isVariable = name.startsWith("--");
this._isVariable = WI.CSSProperty.isVariable(name);
this._styleSheetTextRange = styleSheetTextRange || null;

this._rawValueNewlineIndent = "";
Expand Down Expand Up @@ -576,7 +580,10 @@ WI.CSSProperty = class CSSProperty extends WI.Object
return;

let changeCount = (propertyName, delta) => {
if (!propertyName || this._implicit || this._anonymous || !this._enabled)
if (this._implicit || this._anonymous || !this._enabled)
return;

if (!propertyName || WI.CSSProperty.isVariable(propertyName))
return;

let cachedCount = WI.CSSProperty._cachedNameCounts[propertyName];
Expand Down
2 changes: 1 addition & 1 deletion Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
View file
Open in desktop
Expand Up @@ -358,7 +358,7 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
let variableTokens = tokens.slice(startIndex, i + 1);
startIndex = NaN;

let variableNameIndex = variableTokens.findIndex((token) => token.value.startsWith("--") && /\bvariable-2\b/.test(token.type));
let variableNameIndex = variableTokens.findIndex((token) => WI.CSSProperty.isVariable(token.value) && /\bvariable-2\b/.test(token.type));
if (variableNameIndex === -1)
continue;

Expand Down
2 changes: 1 addition & 1 deletion Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js
View file
Open in desktop
Expand Up @@ -884,7 +884,7 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
continue;

let rawTokens = tokens.slice(startIndex, i + 1);
let variableNameIndex = rawTokens.findIndex((token) => token.value.startsWith("--") && /\bvariable-2\b/.test(token.type));
let variableNameIndex = rawTokens.findIndex((token) => WI.CSSProperty.isVariable(token.value) && /\bvariable-2\b/.test(token.type));
if (variableNameIndex !== -1) {
let contents = rawTokens.slice(0, variableNameIndex + 1);

Expand Down