Changeset 278607 in webkit

Timestamp:

Jun 8, 2021 8:57:00 AM (14 months ago)

Author:

Razvan Caliman

Message:

Web Inspector: Styles panel slow to render when inspecting node with many inherited CSS variables
​https://bugs.webkit.org/show_bug.cgi?id=225972
<rdar://problem/78211185>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Do not show unused inherited CSS variables in the Styles details sidebar.

When aggregating styles for the selected node in WI.DOMNodeStyles, collect a list of names of CSS variables used in CSS property values.
In the Styles details sidebar, skip rendering declarations of inherited CSS variables that are not found in this list.

Always show inherited variables that are used, either directly inherited or via reference (variables using other variables in their value).
Always show inherited variables used as values of inheritable properties like color, font-size, etc.

When a CSS rule contains hidden inherited variables, offer a button to request disclosing them for that rule.
Option-click to show unused inherited variables in all matching rules.

Clicking the "Go to variable" button automatically renders all the unused variables in the CSS rule where the target variable is declared.

• Localizations/en.lproj/localizedStrings.js:
• UserInterface/Models/CSSProperty.js:

(WI.CSSProperty.findVariableNames):

• UserInterface/Models/DOMNodeStyles.js:

(WI.DOMNodeStyles):
(WI.DOMNodeStyles.prototype.get usedCSSVariables):
(WI.DOMNodeStyles.prototype._updateStyleCascade):
(WI.DOMNodeStyles.prototype._collectUsedCSSVariables):

• UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:

(.spreadsheet-style-declaration-editor .property):
(.spreadsheet-style-declaration-editor > .hidden-variables-button):

• UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:

(WI.SpreadsheetCSSStyleDeclarationEditor):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.get propertiesToRender):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.highlightProperty):

• UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:

(WI.SpreadsheetCSSStyleDeclarationSection.prototype.set propertyVisibilityMode):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorSetAllPropertyVisibilityMode):

• UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:

(WI.SpreadsheetRulesStyleDetailsPanel.prototype.spreadsheetCSSStyleDeclarationSectionSetAllPropertyVisibilityMode):

LayoutTests:

Add tests for logic to aggregate used CSS variables in Web Inspector.

• inspector/css/findVariableNames-expected.txt: Added.
• inspector/css/findVariableNames.html: Added.
• inspector/css/usedCSSVariables-expected.txt: Added.
• inspector/css/usedCSSVariables.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/css/findVariableNames-expected.txt (added)
• LayoutTests/inspector/css/findVariableNames.html (added)
• LayoutTests/inspector/css/usedCSSVariables-expected.txt (added)
• LayoutTests/inspector/css/usedCSSVariables.html (added)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js (modified) (2 diffs)
• Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js (modified) (4 diffs)
• Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css (modified) (3 diffs)
• Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js (modified) (8 diffs)
• Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js (modified) (2 diffs)
• Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-06-08  Razvan Caliman  <rcaliman@apple.com>
+
+        Web Inspector: Styles panel slow to render when inspecting node with many inherited CSS variables
+        https://bugs.webkit.org/show_bug.cgi?id=225972
+        <rdar://problem/78211185>
+
+        Reviewed by Devin Rousso.
+
+        Add tests for logic to aggregate used CSS variables in Web Inspector.
+
+        * inspector/css/findVariableNames-expected.txt: Added.
+        * inspector/css/findVariableNames.html: Added.
+        * inspector/css/usedCSSVariables-expected.txt: Added.
+        * inspector/css/usedCSSVariables.html: Added.
+
 2021-06-08  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2021-06-08  Razvan Caliman  <rcaliman@apple.com>
+
+        Web Inspector: Styles panel slow to render when inspecting node with many inherited CSS variables
+        https://bugs.webkit.org/show_bug.cgi?id=225972
+        <rdar://problem/78211185>
+
+        Reviewed by Devin Rousso.
+
+        Do not show unused inherited CSS variables in the Styles details sidebar.
+
+        When aggregating styles for the selected node in `WI.DOMNodeStyles`, collect a list of names of CSS variables used in CSS property values.
+        In the Styles details sidebar, skip rendering declarations of inherited CSS variables that are not found in this list.
+
+        Always show inherited variables that are used, either directly inherited or via reference (variables using other variables in their value).
+        Always show inherited variables used as values of inheritable properties like color, font-size, etc.
+
+        When a CSS rule contains hidden inherited variables, offer a button to request disclosing them for that rule.
+        Option-click to show unused inherited variables in all matching rules.
+
+        Clicking the "Go to variable" button automatically renders all the unused variables in the CSS rule where the target variable is declared.
+
+        * Localizations/en.lproj/localizedStrings.js:
+        * UserInterface/Models/CSSProperty.js:
+        (WI.CSSProperty.findVariableNames):
+        * UserInterface/Models/DOMNodeStyles.js:
+        (WI.DOMNodeStyles):
+        (WI.DOMNodeStyles.prototype.get usedCSSVariables):
+        (WI.DOMNodeStyles.prototype._updateStyleCascade):
+        (WI.DOMNodeStyles.prototype._collectUsedCSSVariables):
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:
+        (.spreadsheet-style-declaration-editor .property):
+        (.spreadsheet-style-declaration-editor > .hidden-variables-button):
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
+        (WI.SpreadsheetCSSStyleDeclarationEditor):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.get propertiesToRender):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.highlightProperty):
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype.set propertyVisibilityMode):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorSetAllPropertyVisibilityMode):
+        * UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:
+        (WI.SpreadsheetRulesStyleDetailsPanel.prototype.spreadsheetCSSStyleDeclarationSectionSetAllPropertyVisibilityMode):
+
 2021-06-04  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

@@ -1029 +1029 @@
 localizedStrings["Open in New Window @ Context Menu Item"] = "Open in New Window";
 localizedStrings["Option-click to show source"] = "Option-click to show source";
+/* Tooltip with instructions on how to show all hidden CSS variables */
+localizedStrings["Option-click to show unused CSS variables from all rules @ Styles Sidebar Panel Tooltip"] = "Option-click to show unused CSS variables from all rules";
 localizedStrings["Options"] = "Options";
 /* Property value for `font-variant-numeric: ordinal`. */
@@ -1320 +1322 @@
 localizedStrings["Shortest property path to %s"] = "Shortest property path to %s";
 localizedStrings["Show %d More"] = "Show %d More";
+/* Text label for button to reveal multiple unused CSS variables */
+localizedStrings["Show %d unused CSS variable (singular) @ Styles Sidebar Panel"] = "Show %d unused CSS variable";
+/* Text label for button to reveal one unused CSS variable */
+localizedStrings["Show %d unused CSS variables (plural) @ Styles Sidebar Panel"] = "Show %d unused CSS variables";
 localizedStrings["Show All"] = "Show All";
 localizedStrings["Show All (%d More)"] = "Show All (%d More)";

trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js

@@ -50 +50 @@
     }
 
+    // FIXME: <https://webkit.org/b/226647> This naively collects variable-like names used in values. It should be hardened.
+    static findVariableNames(string)
+    {
+        const prefix = "var(--";
+        let prefixCursor = 0;
+        let cursor = 0;
+        let nameStartIndex = 0;
+        let names = [];
+
+        function isTerminatingChar(char) {
+            return char === ")" || char === "," || char === " " || char === "\n" || char === "\t";
+        }
+
+        while (cursor < string.length) {
+            if (nameStartIndex && isTerminatingChar(string.charAt(cursor))) {
+                names.push("--" + string.substring(nameStartIndex, cursor));
+                nameStartIndex = 0;
+            }
+
+            if (prefixCursor === prefix.length) {
+                prefixCursor = 0;
+                nameStartIndex = cursor;
+            }
+
+            if (string.charAt(cursor) === prefix.charAt(prefixCursor))
+                prefixCursor++;
+
+            cursor++;
+        }
+
+        return names;
+    }
+
     // Public
 

trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js

@@ -47 +47 @@
 
         this._propertyNameToEffectivePropertyMap = {};
+        this._usedCSSVariables = new Set;
 
         this._pendingRefreshTask = null;
@@ -130 +131 @@
     get orderedStyles() { return this._orderedStyles; }
     get computedPrimaryFont() { return this._computedPrimaryFont; }
+    get usedCSSVariables() { return this._usedCSSVariables; }
 
     get needsRefresh()
@@ -768 +770 @@
         this._associateRelatedProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
         this._markOverriddenProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
+        this._collectUsedCSSVariables(cascadeOrderedStyleDeclarations);
 
         for (let pseudoElementInfo of this._pseudoElements.values()) {
@@ -943 +946 @@
     }
 
+    _collectUsedCSSVariables(styles)
+    {
+        this._usedCSSVariables = new Set;
+
+        for (let style of styles) {
+            for (let property of style.enabledProperties) {
+                let variables = WI.CSSProperty.findVariableNames(property.value);
+
+                if (!style.inherited) {
+                    // FIXME: <https://webkit.org/b/226648> Support the case of variables declared on matching styles but not used anywhere.
+                    this._usedCSSVariables.addAll(variables);
+                    continue;
+                }
+
+                // Always collect variables used in values of inheritable properties.
+                if (WI.CSSKeywordCompletions.InheritedProperties.has(property.name)) {
+                    this._usedCSSVariables.addAll(variables);
+                    continue;
+                }
+
+                // For variables from inherited styles, leverage the fact that styles are already sorted in cascade order to support inherited variables referencing other variables.
+                // If the variable was found to be used before, collect any variables used in its declaration value
+                // (if any variables are found, this isn't the end of the variable reference chain in the inheritance stack).
+                if (property.isVariable && this._usedCSSVariables.has(property.name))
+                    this._usedCSSVariables.addAll(variables);
+            }
+        }
+    }
+
     _isPropertyFoundInMatchingRules(propertyName)
     {

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css

@@ -34 +34 @@
     --background-color-selected: var(--selected-text-background-color);
     --border-color-selected: var(--selected-background-color);
+    --property-checkbox-width: 17px;
+    --property-indentation: calc(var(--css-declaration-horizontal-padding) + var(--property-checkbox-width));
 }
 
@@ -42 +44 @@
 .spreadsheet-style-declaration-editor .property {
     padding-right: var(--css-declaration-horizontal-padding);
-    padding-left: calc(var(--css-declaration-horizontal-padding) + 17px);
+    padding-left: var(--property-indentation);
     border-right: 2px solid transparent;
     border-left: 1px solid transparent;
@@ -232 +234 @@
     left: 16px;
     white-space: nowrap;
+}
+
+.spreadsheet-style-declaration-editor > .hidden-variables-button {
+    margin-left: var(--property-indentation);
 }
 

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js

@@ -41 +41 @@
         this._showsImplicitProperties = false;
         this._alwaysShowPropertyNames = new Set;
-        this._propertyVisibilityMode = WI.SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.ShowAll;
+        this._propertyVisibilityMode = WI.SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.HideUnusedInheritedVariables;
+        this._hiddenUnusedVariables = new Set;
         this._hideFilterNonMatchingProperties = false;
         this._sortPropertiesByName = false;
@@ -105 +106 @@
         }
 
+        if (this._hiddenUnusedVariables.size) {
+            let showHiddenVariablesButtonElement = this.element.appendChild(document.createElement("button"));
+            showHiddenVariablesButtonElement.classList.add("hidden-variables-button");
+            showHiddenVariablesButtonElement.title = WI.UIString("Option-click to show unused CSS variables from all rules", "Option-click to show unused CSS variables from all rules @ Styles Sidebar Panel Tooltip", "Tooltip with instructions on how to show all hidden CSS variables");
+
+            const labelSingular = WI.UIString("Show %d unused CSS variable", "Show %d unused CSS variable (singular) @ Styles Sidebar Panel", "Text label for button to reveal one unused CSS variable");
+            const labelPlural = WI.UIString("Show %d unused CSS variables", "Show %d unused CSS variables (plural) @ Styles Sidebar Panel", "Text label for button to reveal multiple unused CSS variables");
+            let label = this._hiddenUnusedVariables.size > 1 ? labelPlural : labelSingular;
+
+            showHiddenVariablesButtonElement.textContent = label.format(this._hiddenUnusedVariables.size);
+            showHiddenVariablesButtonElement.addEventListener("click", (event) => {
+                if (event.altKey) {
+                    this._setAllPropertyVisibilityMode(WI.SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.ShowAll);
+                    return;
+                }
+
+                this.propertyVisibilityMode = WI.SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.ShowAll;
+            });
+        }
+
         if (propertyViewPendingStartEditing) {
             propertyViewPendingStartEditing.startEditingName();
@@ -118 +139 @@
             this.selectProperties(this._anchorIndex, this._focusIndex);
 
+        if (this._pendingPropertyToHighlight) {
+            this.highlightProperty(this._pendingPropertyToHighlight);
+            this._pendingPropertyToHighlight = null;
+        }
+
         this._updateDebugLockStatus();
     }
@@ -235 +261 @@
         let hideVariables = this._propertyVisibilityMode === SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.HideVariables;
         let hideNonVariables = this._propertyVisibilityMode === SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.HideNonVariables;
+        let hideUnusedInheritedVariables = this._propertyVisibilityMode === SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.HideUnusedInheritedVariables;
+
+        this._hiddenUnusedVariables = new Set;
 
         return properties.filter((property) => {
@@ -242 +271 @@
             if (property.isVariable && hideVariables)
                 return false;
+
+            if (property.isVariable && hideUnusedInheritedVariables && this._style.inherited && !this._style.nodeStyles.usedCSSVariables.has(property.name)) {
+                this._hiddenUnusedVariables.add(property);
+                return false;
+            }
 
             return !property.implicit || this._showsImplicitProperties || this._alwaysShowPropertyNames.has(property.canonicalName);
@@ -278 +312 @@
     highlightProperty(property)
     {
+        if (!property.overridden && this._hiddenUnusedVariables.has(property)) {
+            this._pendingPropertyToHighlight = property;
+            this.propertyVisibilityMode = WI.SpreadsheetCSSStyleDeclarationEditor.PropertyVisibilityMode.ShowAll;
+            return true;
+        }
+
         let propertiesMatch = (cssProperty) => {
             if (cssProperty.attached && !cssProperty.overridden) {
@@ -684 +724 @@
         this.element.classList.toggle("debug-style-locked", this._style.locked);
     }
+
+    _setAllPropertyVisibilityMode(propertyVisibilityMode)
+    {
+        this._delegate?.spreadsheetCSSStyleDeclarationEditorSetAllPropertyVisibilityMode?.(this, propertyVisibilityMode);
+    }
 };
 
@@ -696 +741 @@
     HideVariables: Symbol("variable-visibility-hide-variables"),
     HideNonVariables: Symbol("variable-visibility-hide-non-variables"),
+    HideUnusedInheritedVariables: Symbol("variable-visibility-hide-unused-inherited-variables"),
 };

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js

@@ -59 +59 @@
     }
 
+    set propertyVisibilityMode(propertyVisibilityMode)
+    {
+        this._propertiesEditor.propertyVisibilityMode = propertyVisibilityMode;
+    }
+
     initialLayout()
     {
@@ -285 +290 @@
     }
 
+    spreadsheetCSSStyleDeclarationEditorSetAllPropertyVisibilityMode(editor, propertyVisibilityMode)
+    {
+        this._delegate?.spreadsheetCSSStyleDeclarationSectionSetAllPropertyVisibilityMode?.(this, propertyVisibilityMode);
+    }
+
     applyFilter(filterText)
     {

trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js

@@ -204 +204 @@
     }
 
+    spreadsheetCSSStyleDeclarationSectionSetAllPropertyVisibilityMode(section, propertyVisibilityMode)
+    {
+        for (let section of this._sections)
+            section.propertyVisibilityMode = propertyVisibilityMode;
+    }
+
     // Protected