commit 02dc395a6fa097e3137200469605b9e20fc9e16d parent 6618651b9919d280614642b886db337f3d2f48bb Author: Nicolas Chevobbe <nchevobbe@mozilla.com> Date: Sat, 18 Oct 2025 14:17:38 +0000 Bug 1719461 - [devtools] Hide CSS variables that are not used in regular properties. r=devtools-reviewers,jdescottes. Differential Revision: https://phabricator.services.mozilla.com/D268086 Diffstat:
16 files changed, 1628 insertions(+), 24 deletions(-)
diff --git a/devtools/client/inspector/rules/models/element-style.js b/devtools/client/inspector/rules/models/element-style.js @@ -454,6 +454,15 @@ class ElementStyle { this.variablesMap.set(pseudo, variables); this.startingStyleVariablesMap.set(pseudo, startingStyleVariables); + const rulesEditors = new Set(); + const variableTree = new Map(); + + if (!this.usedVariables) { + this.usedVariables = new Set(); + } else { + this.usedVariables.clear(); + } + // For each TextProperty, mark it overridden if all of its computed // properties are marked overridden. Update the text property's associated // editor, if any. This will clear the _overriddenDirty state on all @@ -479,6 +488,55 @@ class ElementStyle { if (textProp.editor) { textProp.editor.updateUI(); } + + // First we need to update used variables from all declarations + textProp.updateUsedVariables(); + const isCustomProperty = textProp.name.startsWith("--"); + const isNewCustomProperty = + isCustomProperty && textProp.isPropertyChanged; + if (isNewCustomProperty) { + this.usedVariables.add(textProp.name); + } + if (textProp.usedVariables) { + if (!isCustomProperty) { + for (const variable of textProp.usedVariables) { + this.usedVariables.add(variable); + } + } else { + variableTree.set(textProp.name, textProp.usedVariables); + } + } + + if (textProp.rule.editor) { + rulesEditors.add(textProp.rule.editor); + } + } + + const collectVariableDependencies = variable => { + if (!variableTree.has(variable)) { + return; + } + + for (const dep of variableTree.get(variable)) { + if (!this.usedVariables.has(dep)) { + this.usedVariables.add(dep); + collectVariableDependencies(dep); + } + } + }; + + for (const variable of this.usedVariables) { + collectVariableDependencies(variable); + } + + for (const textProp of textProps) { + // Then we need to update the isUnusedCssVariable + textProp.updateIsUnusedVariable(); + } + + // Then update the UI + for (const ruleEditor of rulesEditors) { + ruleEditor.updateUnusedCssVariables(); } } diff --git a/devtools/client/inspector/rules/models/rule.js b/devtools/client/inspector/rules/models/rule.js @@ -811,7 +811,8 @@ class Rule { if (direction === Services.focus.MOVEFOCUS_FORWARD) { for (++index; index < this.textProps.length; ++index) { - if (!this.textProps[index].invisible) { + // The prop could be invisible or a hidden unused variable + if (this.textProps[index].editor) { break; } } @@ -822,7 +823,8 @@ class Rule { } } else if (direction === Services.focus.MOVEFOCUS_BACKWARD) { for (--index; index >= 0; --index) { - if (!this.textProps[index].invisible) { + // The prop could be invisible or a hidden unused variable + if (this.textProps[index].editor) { break; } } diff --git a/devtools/client/inspector/rules/models/text-property.js b/devtools/client/inspector/rules/models/text-property.js @@ -76,6 +76,7 @@ class TextProperty { this.updateComputed(); this.updateUsedVariables(); + this.updateIsUnusedVariable(); } get computedProperties() { @@ -170,6 +171,22 @@ class TextProperty { } /** + * Sets this.isUnusedVariable + */ + updateIsUnusedVariable() { + this.isUnusedVariable = + this.name.startsWith("--") && + // If an editor was created for the declaration, never hide it back + !this.editor && + // Don't consider user-added variables, custom properties whose name is the same as + // user-added variables, to be unused (we do want to display those to avoid confusion + // for the user. + !this.userProperties.containsName(this.name) && + this.elementStyle.usedVariables && + !this.elementStyle.usedVariables.has(this.name); + } + + /** * Set all the values from another TextProperty instance into * this TextProperty instance. * diff --git a/devtools/client/inspector/rules/models/user-properties.js b/devtools/client/inspector/rules/models/user-properties.js @@ -13,6 +13,8 @@ class UserProperties { this.map = new Map(); } + #propertyNames = new Set(); + /** * Get a named property for a given CSSStyleDeclaration. * @@ -57,6 +59,7 @@ class UserProperties { props[name] = userValue; this.map.set(key, props); } + this.#propertyNames.add(name); } /** @@ -73,12 +76,23 @@ class UserProperties { return !!entry && name in entry; } + /** + * Check whether a named property is stored. + * + * @param {String} name + * The name of the property to check. + */ + containsName(name) { + return this.#propertyNames.has(name); + } + getKey(style, name) { return style.actorID + ":" + name; } clear() { this.map.clear(); + this.#propertyNames.clear(); } } diff --git a/devtools/client/inspector/rules/rules.js b/devtools/client/inspector/rules/rules.js @@ -150,7 +150,9 @@ function CssRuleView(inspector, document, store) { this.cssProperties = inspector.cssProperties; this.styleDocument = document; this.styleWindow = this.styleDocument.defaultView; - this.store = store || {}; + this.store = store || { + expandedUnusedCustomCssPropertiesRuleActorIds: new Set(), + }; // Allow tests to override debouncing behavior, as this can cause intermittents. this.debounce = debounce; @@ -1500,8 +1502,18 @@ CssRuleView.prototype = { // Initialize rule editor if (!rule.editor) { + const ruleActorID = rule.domRule.actorID; rule.editor = new RuleEditor(this, rule, { elementsWithPendingClicks: this._elementsWithPendingClicks, + onShowUnusedCustomCssProperties: () => { + this.store.expandedUnusedCustomCssPropertiesRuleActorIds.add( + ruleActorID + ); + }, + shouldHideUnusedCustomCssProperties: + !this.store.expandedUnusedCustomCssPropertiesRuleActorIds.has( + ruleActorID + ), }); editorReadyPromises.push(rule.editor.once("source-link-updated")); } @@ -1809,15 +1821,18 @@ CssRuleView.prototype = { * false otherwise. */ _highlightRuleProperty(textProperty) { - // Get the actual property value displayed in the rule view - const propertyName = textProperty.editor.prop.name.toLowerCase(); - const propertyValue = - textProperty.editor.valueSpan.textContent.toLowerCase(); + const propertyName = textProperty.name.toLowerCase(); + // Get the actual property value displayed in the rule view if we have an editor for + // it (that might not be the case for unused CSS custom properties). + const propertyValue = textProperty.editor + ? textProperty.editor.valueSpan.textContent.toLowerCase() + : textProperty.value.toLowerCase(); return this._highlightMatches({ - element: textProperty.editor.container, + element: textProperty.editor?.container, propertyName, propertyValue, + textProperty, }); }, @@ -1832,6 +1847,10 @@ CssRuleView.prototype = { * otherwise. */ _highlightComputedProperty(textProperty) { + if (!textProperty.editor) { + return false; + } + let isComputedHighlighted = false; // Highlight search matches in the computed list of properties @@ -1846,6 +1865,7 @@ CssRuleView.prototype = { element: computed.element, propertyName: computedName, propertyValue: computedValue, + textProperty, }) ? true : isComputedHighlighted; @@ -1867,10 +1887,13 @@ CssRuleView.prototype = { * The property name of a rule * @param {String} options.propertyValue * The property value of a rule + * @param {TextProperty} options.textProperty + * The text property that we may highlight. It's helpful in cases we don't have + * an element yet (e.g. if the property is a hidden unused variable) * @return {Boolean} true if the given search terms match the property, false * otherwise. */ - _highlightMatches({ element, propertyName, propertyValue }) { + _highlightMatches({ element, propertyName, propertyValue, textProperty }) { const { searchPropertyName, searchPropertyValue, @@ -1909,11 +1932,26 @@ CssRuleView.prototype = { ); } - if (matches) { - element.classList.add("ruleview-highlight"); + if (!matches) { + return false; } - return matches; + // We might not have an element when the prop is an unused custom css property. + if (!element && textProperty?.isUnusedVariable) { + const editor = + textProperty.rule.editor.showUnusedCssVariable(textProperty); + + // The editor couldn't be created, bail (shouldn't happen) + if (!editor) { + return false; + } + + element = editor.container; + } + + element.classList.add("ruleview-highlight"); + + return true; }, /** @@ -2200,6 +2238,12 @@ CssRuleView.prototype = { this._togglePseudoElementRuleContainer(); } + // If we're jumping to an unused CSS variable, it might not be visible, so show + // it here. + if (!textProp.editor && textProp.isUnusedVariable) { + textProp.rule.editor.showUnusedCssVariable(textProp); + } + this._highlightElementInRule( rule, textProp.editor.element, @@ -2717,6 +2761,7 @@ class RuleViewTool { this.document = this.inspector = this.readyPromise = + this.store = this.#abortController = null; } diff --git a/devtools/client/inspector/rules/test/browser_part1.toml b/devtools/client/inspector/rules/test/browser_part1.toml @@ -319,3 +319,9 @@ skip-if = ["os == 'linux' && os_version == '24.04' && processor == 'x86_64' && d ["browser_rules_variables_autocomplete.js"] ["browser_rules_variables_host.js"] + +["browser_rules_variables_unused_add_property.js"] + +["browser_rules_variables_unused_filtering.js"] + +["browser_rules_variables_unused.js"] diff --git a/devtools/client/inspector/rules/test/browser_rules_copy_styles.js b/devtools/client/inspector/rules/test/browser_rules_copy_styles.js @@ -112,6 +112,27 @@ add_task(async function () { }, }, { + desc: "Test Copy Rule with hidden unused variables", + node: getRuleViewRuleEditor(view, 2).rule.textProps[0].editor.nameSpan, + menuItemLabel: "styleinspector.contextmenu.copyRule", + expectedPattern: + ":where\\(#testid\\) {[\\r\\n]+" + + "\tcolor: gold;[\\r\\n]+" + + Array.from({ length: 13 }, (_, i) => { + return `\t--unused-${i + 1}: ${i + 1};[\\r\\n]+`; + }).join("") + + "\tbackground-color: tomato;[\\r\\n]+" + + "}", + visible: { + copyLocation: false, + copyDeclaration: true, + copyPropertyName: true, + copyPropertyValue: false, + copySelector: false, + copyRule: true, + }, + }, + { desc: "Test Copy Selector", node: ruleEditor.selectorText, menuItemLabel: "styleinspector.contextmenu.copySelector", diff --git a/devtools/client/inspector/rules/test/browser_rules_variables-jump-to-definition.js b/devtools/client/inspector/rules/test/browser_rules_variables-jump-to-definition.js @@ -279,8 +279,9 @@ add_task(async function () { add_task(async function checkClearSearch() { const fillerDeclarations = Array.from({ length: 50 }, (_, i) => ({ - name: `--x-${i}`, + name: `line-height`, value: i.toString(), + overridden: i !== 49, })); await addTab( @@ -360,6 +361,87 @@ add_task(async function checkClearSearch() { ]); }); +add_task(async function checkJumpToUnusedVariable() { + await addTab( + "data:text/html;charset=utf-8," + + encodeURIComponent(` + <style> + :where(h3) { + ${Array.from({ length: 15 }, (_, i) => `--unused-${i}: ${i};`).join("\n")} + } + + h3 { + --another-unused: var(--unused-5); + } + </style> + <h3>for unused variables</h3> + `) + ); + + const { inspector, view } = await openRuleView(); + await selectNode("h1", inspector); + + info("Check that jump to definition of unused variables do work"); + // If you have 2 rules, one with hidden custom properties, and the other one with + // custom properties not being hidden because we're not entering the threshold + // Make sure the clicking the Jump to definition button will reveal the hidden property + await selectNode("h3", inspector); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h3", + declarations: [{ name: "--another-unused", value: "var(--unused-5)" }], + }, + { + // Contains the hidden variables + selector: ":where(h3)", + // All the variables are hidden + declarations: [], + }, + ]); + + is( + getUnusedVariableButton(view, 2)?.textContent, + "Show 15 unused custom CSS properties", + "Show unused variables button has expected text" + ); + + const rule = getRuleViewRuleEditor(view, 1).rule; + is(rule.selectorText, "h3", "Got expected rule"); + + const variableButtonEls = getJumpToDefinitionButtonForDeclaration(rule, { + "--another-unused": "var(--unused-5)", + }); + is(variableButtonEls.length, 1, "There's one jump to variable button"); + await highlightProperty(view, variableButtonEls[0], "--unused-5", "5"); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h3", + declarations: [{ name: "--another-unused", value: "var(--unused-5)" }], + }, + { + // Contains the hidden variables + selector: ":where(h3)", + declarations: [{ name: "--unused-5", value: "5" }], + }, + ]); + + is( + getUnusedVariableButton(view, 2)?.textContent, + "Show 14 unused custom CSS properties", + "Show unused variables button has expected text" + ); +}); + function getJumpToDefinitionButtonForDeclaration(rule, declaration) { const [[name, value]] = Object.entries(declaration); const textProp = rule.textProps.find(prop => { diff --git a/devtools/client/inspector/rules/test/browser_rules_variables_unused.js b/devtools/client/inspector/rules/test/browser_rules_variables_unused.js @@ -0,0 +1,239 @@ +/* Any copyright is dedicated to the Public Domain. + https://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function testHiddenUnusedVariables() { + const h1Declarations = [ + { name: "--foo", value: "1" }, + { name: "--bar", value: "2" }, + { name: "--foobar", value: "calc( var(--foo, 3) * var(--bar, 4))" }, + { name: "--fallback", value: "var(--fallback-a)" }, + { name: "--fallback-a", value: "var(--fallback-b)" }, + { name: "--fallback-b", value: "var(--fallback-c)" }, + { name: "--fallback-c", value: "10" }, + { name: "--cycle-a", value: "var(--cycle-b)" }, + { name: "--cycle-b", value: "var(--cycle-a)" }, + { name: "--unused-a", value: "var(--unused-b)" }, + { name: "--unused-b", value: "5" }, + { name: "--h", value: "400px" }, + // Generate a good amount of variables that won't be referenced anywhere to trigger the + // "hide unused" mechanism + ...Array.from({ length: 10 }, (_, i) => ({ + name: `--unused-no-dep-${i}`, + value: i.toString(), + })), + { + name: "width", + value: `calc(var(--foobar, var(--fallback)) + var(--cycle-a) + var(--unset))`, + }, + ]; + + // set a different rule using a variable from the first rule to check if its detected + // as being used + const whereH1Declarations = [ + // declare 9 unused variables, so they should be visible by default + ...Array.from({ length: 9 }, (_, i) => ({ + name: `--unused-where-${i}`, + value: i.toString(), + })), + { + name: "height", + // Using variable for the h1 rule + value: "var(--h)", + }, + ]; + + const TEST_URI = ` + <style> + h1 { + ${h1Declarations + .map(({ name, value }) => `${name}: ${value};`) + .join("\n")} + } + + :where(h1) { + ${whereH1Declarations + .map(({ name, value }) => `${name}: ${value};`) + .join("\n")} + } + </style> + <h1>Hello</h1> +`; + + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + const { inspector, view } = await openRuleView(); + await selectNode("h1", inspector); + + info("Check that elementStyle.usedVariables has the expected data"); + Assert.deepEqual(Array.from(view._elementStyle.usedVariables), [ + // in `h1 -> width` + "--foobar", + // in `h1 -> width` + "--fallback", + // in `h1 -> width` + "--cycle-a", + // in `h1 -> width`, is picked up even if it's not defined + "--unset", + // in `:where(h1) -> height` + "--h", + // in `h1 -> --foobar`, which is used in `h1 -> width` + "--foo", + // in `h1 -> --foobar`, which is used in `h1 -> width` + "--bar", + // in `h1 -> --fallback`, which is used in `h1 -> width` + "--fallback-a", + // in `h1 -> --fallback-a`, which is used in `h1 -> --fallback`, which is used in `h1 -> width` + "--fallback-b", + // in `h1 -> --fallback-b`, which is used in `h1 -> --fallback-a`, which is used in `h1 -> --fallback`, + // which is used in `h1 -> width` + "--fallback-c", + // in `h1 --cycle-a`, which is used in `h1 -> width` + "--cycle-b", + ]); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { name: "--foo", value: "1" }, + { name: "--bar", value: "2" }, + { name: "--foobar", value: "calc( var(--foo, 3) * var(--bar, 4))" }, + { name: "--fallback", value: "var(--fallback-a)" }, + { name: "--fallback-a", value: "var(--fallback-b)" }, + { name: "--fallback-b", value: "var(--fallback-c)" }, + { name: "--fallback-c", value: "10" }, + { name: "--cycle-a", value: "var(--cycle-b)" }, + { name: "--cycle-b", value: "var(--cycle-a)" }, + // Displayed because used in `:where(h1) -> height` + { name: "--h", value: "400px" }, + { + name: "width", + value: `calc(var(--foobar, var(--fallback)) + var(--cycle-a) + var(--unset))`, + }, + ], + }, + { + selector: ":where(h1)", + // All declarations are displayed, even the unused variables, because we don't + // hit the threshold to trigger the "hide unused" UI + declarations: whereH1Declarations, + }, + ]); + + info("Check that the 'Show X unused variables button is displayed'"); + const showUnusedVariablesButton = getUnusedVariableButton(view, 1); + ok(!!showUnusedVariablesButton, "Show unused variables button is displayed"); + + info("Check that the button doesn't prevent the usual keyboard navigation"); + const h1RuleEditor = getRuleViewRuleEditor(view, 1); + const whereH1RuleEditor = getRuleViewRuleEditor(view, 2); + + await focusNewRuleViewProperty(h1RuleEditor); + + EventUtils.synthesizeKey("VK_TAB", {}, view.styleWindow); + is( + inplaceEditor(view.styleDocument.activeElement), + inplaceEditor(whereH1RuleEditor.selectorText), + "Hitting Tab triggered the editor for the selector of the next rule" + ); + + EventUtils.synthesizeKey("VK_TAB", { shiftKey: true }, view.styleWindow); + is( + inplaceEditor(view.styleDocument.activeElement), + inplaceEditor(h1RuleEditor.newPropSpan), + "Hitting Shift+Tab triggered the editor for the new property" + ); + + // Blur the input to not interfere with the rest of the test + const onBlur = once(view.styleDocument.activeElement, "blur"); + view.styleDocument.activeElement.blur(); + await onBlur; + + info( + "Check that clicking the Show unused variable button does show the unused variables" + ); + showUnusedVariablesButton.click(); + + is( + getUnusedVariableButton(view, 1), + null, + "Show unused variable button is not visible anymore" + ); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: h1Declarations, + }, + { + selector: ":where(h1)", + declarations: whereH1Declarations, + }, + ]); + + info( + "Selecting another node and select h1 back to assert the rules after a refresh" + ); + await selectNode("body", inspector); + await selectNode("h1", inspector); + + is( + getUnusedVariableButton(view, 1), + null, + "Unused variable button is kept hidden after refreshing rules view" + ); + + info("Add another unused variables to the :where(h1) rule"); + // Sanity check + ok( + !getUnusedVariableButton(view, 2), + "The unused variable button isn't displayed at first for :where(h1) rule" + ); + + // We shouldn't add the property via the UI, as variables added by the user in the + // Rules view are always visible (see browser_rules_variables_unused_add_property.js). + // We could add the property via CSSOM, but it looks like there's a bug at the moment + // where properties aren't showing up (unrelated to unused variable). + // So add the property via the UI, but clear the user properties so it won't be seen + // as added by the user. + await addProperty(view, 2, "--added-unused-where", "new-1"); + view.store.userProperties.clear(); + + info( + "Selecting another node and select h1 back after adding property via CSSOM" + ); + await selectNode("body", inspector); + await selectNode("h1", inspector); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: h1Declarations, + }, + { + selector: ":where(h1)", + // we only see the height variable, --unused-c is now hidden, as well as all the + // --unused-cssom-* variables + declarations: [{ name: "height", value: "var(--h)" }], + }, + ]); + + is( + getUnusedVariableButton(view, 2).textContent, + "Show 10 unused custom CSS properties", + "Unused variable button is kept hidden after refreshing rules view" + ); +}); diff --git a/devtools/client/inspector/rules/test/browser_rules_variables_unused_add_property.js b/devtools/client/inspector/rules/test/browser_rules_variables_unused_add_property.js @@ -0,0 +1,611 @@ +/* Any copyright is dedicated to the Public Domain. + https://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function testUnusedVariablesAndAddingDeclarations() { + const declarations = [ + // Generate a good amount of variables that won't be referenced anywhere so + // we to trigger the "hide unused" mechanism + ...Array.from({ length: 15 }, (_, i) => ({ + name: `--unused-${i}`, + value: i.toString(), + })), + ]; + + const URI = ` + <style> + h1 { + width: auto; + ${declarations.map(({ name, value }) => `${name}: ${value};`).join("\n")} + } + </style> + <h1>Unused variable / Add declarations</h1> +`; + + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(URI)); + const { inspector, view } = await openRuleView(); + await selectNode("h1", inspector); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 15 unused custom CSS properties", + "Show unused variables has expected text" + ); + + info( + "Check that when adding a new unused CSS property it will never be hidden" + ); + await addProperty(view, 1, "--added-unused", "new-1"); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { name: "width", value: "auto" }, + { name: "--added-unused", value: "new-1", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 15 unused custom CSS properties", + "Show unused variables has expected text after adding a new unused property" + ); + + info( + "Select the body element and then h1 again to make sure the variable is still visible" + ); + await selectNode("body", inspector); + await selectNode("h1", inspector); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { name: "width", value: "auto" }, + { name: "--added-unused", value: "new-1", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 15 unused custom CSS properties", + "Show unused variables still has expected text after adding a new unused property and navigating to different nodes" + ); + + info("Check that unused properties with the same name are made visible"); + await addProperty(view, 1, "--unused-5", "new-2"); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { name: "width", value: "auto" }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 14 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declaration is now visible" + ); + + info("Check that adding a property reveals the now used variables"); + await addProperty(view, 1, "color", "var(--unused-1, var(--unused-9))"); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { name: "width", value: "auto" }, + { name: "--unused-1", value: "1" }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 12 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declarations are now visible" + ); + + info("Check that editing a property reveals the now used variables"); + const widthProp = getTextProperty(view, 1, { width: "auto" }); + await setProperty(view, widthProp, "var(--unused-2, var(--unused-11))"); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "width", + value: "var(--unused-2, var(--unused-11))", + dirty: true, + }, + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-11", value: "11" }, + { + name: "--added-unused", + value: "new-1", + dirty: true, + }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 10 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declarations are now visible" + ); + + info( + "Check that adding empty variable does show unused variable with the same name" + ); + await addProperty(view, 1, "--unused-6", ""); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "width", + value: "var(--unused-2, var(--unused-11))", + dirty: true, + }, + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-6", + value: "6", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-11", value: "11" }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 9 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declarations are now visible" + ); + + info( + "Check that adding multiple declarations at once properly detects all the new used variables" + ); + const ruleEditor = getRuleViewRuleEditor(view, 1); + const onRuleViewChanged = view.once("ruleview-changed"); + await createNewRuleViewProperty( + ruleEditor, + "--unused-8: new-8; --unused-12: new-12;" + ); + await onRuleViewChanged; + + // createNewRuleViewProperty focuses the new property input, so blur here + const onBlur = once(view.styleDocument.activeElement, "blur"); + view.styleDocument.activeElement.blur(); + await onBlur; + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "width", + value: "var(--unused-2, var(--unused-11))", + dirty: true, + }, + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-6", + value: "6", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-8", + value: "8", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-11", value: "11" }, + { + name: "--unused-12", + value: "12", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + { name: "--unused-8", value: "new-8", dirty: true }, + { name: "--unused-12", value: "new-12", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 7 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declarations are now visible" + ); + + info( + "Check that adding declarations in the style rule won't show all the variables" + ); + await addProperty(view, 0, "--unused-4", "new-4"); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [{ name: "--unused-4", value: "new-4", dirty: true }], + }, + { + selector: "h1", + declarations: [ + { + name: "width", + value: "var(--unused-2, var(--unused-11))", + dirty: true, + }, + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { + name: "--unused-4", + value: "4", + overridden: true, + }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-6", + value: "6", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-8", + value: "8", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-11", value: "11" }, + { + name: "--unused-12", + value: "12", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + { name: "--unused-8", value: "new-8", dirty: true }, + { name: "--unused-12", value: "new-12", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 6 unused custom CSS properties", + "Show unused variables text was updated as previously hidden declarations are now visible" + ); + + info("Check that removing declaration (e.g. width) does not hide variables"); + await removeProperty( + view, + getTextProperty(view, 1, { width: "var(--unused-2, var(--unused-11))" }) + ); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [{ name: "--unused-4", value: "new-4", dirty: true }], + }, + { + selector: "h1", + declarations: [ + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { name: "--unused-4", value: "4", overridden: true }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-6", + value: "6", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-8", + value: "8", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-11", value: "11" }, + { + name: "--unused-12", + value: "12", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + { name: "--unused-8", value: "new-8", dirty: true }, + { name: "--unused-12", value: "new-12", dirty: true }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 6 unused custom CSS properties", + "Show unused variables text wasn't updated after removing a property using variables" + ); + + // and then that adding a new property at the end using one of the variable will show + // it at the right position (update declarationIndex) + info( + "Check that adding declaration using variables insert them at the right place, even after a declaration was removed" + ); + await addProperty( + view, + 1, + "height", + "calc(var(--unused-3) + var(--unused-10) * 1px)" + ); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [{ name: "--unused-4", value: "new-4", dirty: true }], + }, + { + selector: "h1", + declarations: [ + { name: "--unused-1", value: "1" }, + { name: "--unused-2", value: "2" }, + { name: "--unused-3", value: "3" }, + { name: "--unused-4", value: "4", overridden: true }, + { + name: "--unused-5", + value: "5", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-6", + value: "6", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { + name: "--unused-8", + value: "8", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--unused-9", value: "9" }, + { name: "--unused-10", value: "10" }, + { name: "--unused-11", value: "11" }, + { + name: "--unused-12", + value: "12", + overridden: true, + // Shouldn't really (see Bug 1993440) + dirty: true, + }, + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + { name: "--unused-8", value: "new-8", dirty: true }, + { name: "--unused-12", value: "new-12", dirty: true }, + { + name: "height", + value: "calc(var(--unused-3) + var(--unused-10) * 1px)", + dirty: true, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 4 unused custom CSS properties", + "Show unused variables text was updated" + ); + + info("Check that clicking the show unused variable button works as expected"); + getUnusedVariableButton(view, 1).click(); + + is( + getUnusedVariableButton(view, 1), + null, + "Show unused variable button is not visible anymore" + ); + + const overriddenDeclarations = [ + "--unused-4", + "--unused-5", + "--unused-6", + "--unused-8", + "--unused-12", + ]; + // Those shouldn't be tagged as dirty but are (see Bug 1993440) + const dirtyDeclarations = [ + "--unused-5", + "--unused-6", + "--unused-8", + "--unused-12", + ]; + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [{ name: "--unused-4", value: "new-4", dirty: true }], + }, + { + selector: "h1", + declarations: [ + ...declarations.map(decl => ({ + ...decl, + overridden: overriddenDeclarations.includes(decl.name), + dirty: dirtyDeclarations.includes(decl.name), + })), + { name: "--added-unused", value: "new-1", dirty: true }, + { name: "--unused-5", value: "new-2", dirty: true }, + { + name: "color", + value: "var(--unused-1, var(--unused-9))", + dirty: true, + }, + { name: "--unused-6", value: "", dirty: true }, + { name: "--unused-8", value: "new-8", dirty: true }, + { name: "--unused-12", value: "new-12", dirty: true }, + { + name: "height", + value: "calc(var(--unused-3) + var(--unused-10) * 1px)", + dirty: true, + }, + ], + }, + ]); +}); diff --git a/devtools/client/inspector/rules/test/browser_rules_variables_unused_filtering.js b/devtools/client/inspector/rules/test/browser_rules_variables_unused_filtering.js @@ -0,0 +1,229 @@ +/* Any copyright is dedicated to the Public Domain. + https://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function testUnusedVariablesAndFiltering() { + const declarations = [ + { + name: "--my-var-1", + value: `a`, + }, + { + name: "--my-var-2", + value: `b`, + }, + // Generate a good amount of variables that won't be referenced anywhere to trigger the + // "hide unused" mechanism + ...Array.from({ length: 10 }, (_, i) => ({ + name: `--unused-${i}`, + value: i.toString(), + })), + { + name: "--x", + value: `hotpink`, + }, + ]; + + const TEST_URI = ` + <style> + h1 { + ${declarations.map(({ name, value }) => `${name}: ${value};`).join("\n")} + } + </style> + <h1>Hello</h1> +`; + + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + const { inspector, view } = await openRuleView(); + await selectNode("h1", inspector); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [], + }, + ]); + is( + getUnusedVariableButton(view, 1).textContent, + "Show 13 unused custom CSS properties", + "Show unused variables has expected text" + ); + + info("Check that searching for hidden variables show them"); + await setSearchFilter(view, "--my-var"); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "--my-var-1", + value: `a`, + highlighted: true, + }, + { + name: "--my-var-2", + value: `b`, + highlighted: true, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 11 unused custom CSS properties", + "Show unused variables has expected text" + ); + + info("Check that clearing the filter doesn't hide the results back"); + const searchClearButton = view.searchClearButton; + + info("Clearing the search filter"); + const onRuleviewFiltered = view.inspector.once("ruleview-filtered"); + EventUtils.synthesizeMouseAtCenter(searchClearButton, {}, view.styleWindow); + await onRuleviewFiltered; + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "--my-var-1", + value: `a`, + highlighted: false, + }, + { + name: "--my-var-2", + value: `b`, + highlighted: false, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 11 unused custom CSS properties", + "Show unused variables has expected text" + ); + + info("Check that searching for value of hidden variables do show them"); + await setSearchFilter(view, "pink"); + + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: [ + { + name: "--my-var-1", + value: `a`, + highlighted: false, + }, + { + name: "--my-var-2", + value: `b`, + highlighted: false, + }, + { + name: "--x", + value: `hotpink`, + highlighted: true, + }, + ], + }, + ]); + + is( + getUnusedVariableButton(view, 1).textContent, + "Show 10 unused custom CSS properties", + "Show unused variables has expected text" + ); + + info("Check that Tab key navigation works as expected"); + // Here we're in a state where we have hidden variables that should be between + // --my-var-2 and --x (all the --unused-* ones). + // Since keyboard navigation with Tab between editor is based on the declarations, let's + // check that we properly navigate with the visible items + + const myVar2Prop = getTextProperty(view, 1, { "--my-var-2": "b" }); + const myXProp = getTextProperty(view, 1, { "--x": "hotpink" }); + + info("focus --my-var-2 value"); + await focusEditableField(view, myVar2Prop.editor.valueSpan); + + info("hit tab"); + const ruleEditor = getRuleViewRuleEditor(view, 1); + let onFocus = once(ruleEditor.element, "focus", true); + let onRuleViewChanged = view.once("ruleview-changed"); + EventUtils.synthesizeKey("VK_TAB", {}, view.styleWindow); + await onFocus; + await onRuleViewChanged; + + is( + inplaceEditor(view.styleDocument.activeElement), + inplaceEditor(myXProp.editor.nameSpan), + "--x name input is focused" + ); + + info("hit shift + tab"); + onFocus = once(ruleEditor.element, "focus", true); + EventUtils.synthesizeKey("VK_TAB", { shiftKey: true }, view.styleWindow); + await onFocus; + + is( + inplaceEditor(view.styleDocument.activeElement), + inplaceEditor(myVar2Prop.editor.valueSpan), + "--my-var-2 value input is focused" + ); + + // Blur the field to put back the UI in its initial state (and avoid pending + // requests when the test ends). + onRuleViewChanged = view.once("ruleview-changed"); + EventUtils.synthesizeKey("VK_ESCAPE", {}, view.styleWindow); + view.debounce.flush(); + await onRuleViewChanged; + + info( + "Check that clicking on the Show unused button does show all the variables at the expected positions" + ); + getUnusedVariableButton(view, 1).click(); + + is( + getUnusedVariableButton(view, 1), + null, + "Show unused variable button is not visible anymore" + ); + await checkRuleViewContent(view, [ + { + selector: "element", + declarations: [], + }, + { + selector: "h1", + declarations: declarations.map(({ name, value }) => ({ + name, + value, + // We didn't clear the search, --x should still be highlighted + highlighted: name === "--x", + })), + }, + ]); +}); diff --git a/devtools/client/inspector/rules/test/doc_copystyles.css b/devtools/client/inspector/rules/test/doc_copystyles.css @@ -9,3 +9,21 @@ html, body, #testid { border-color: #00F !important; --var: "*/"; } + +:where(#testid) { + color: gold; + --unused-1: 1; + --unused-2: 2; + --unused-3: 3; + --unused-4: 4; + --unused-5: 5; + --unused-6: 6; + --unused-7: 7; + --unused-8: 8; + --unused-9: 9; + --unused-10: 10; + --unused-11: 11; + --unused-12: 12; + --unused-13: 13; + background-color: tomato; +} diff --git a/devtools/client/inspector/rules/test/head.js b/devtools/client/inspector/rules/test/head.js @@ -1494,3 +1494,9 @@ function checkRuleViewContent(view, expectedElements) { } } } + +function getUnusedVariableButton(view, elementIndexInView) { + return view.element.children[elementIndexInView].querySelector( + ".ruleview-show-unused-custom-css-properties" + ); +} diff --git a/devtools/client/inspector/rules/views/rule-editor.js b/devtools/client/inspector/rules/views/rule-editor.js @@ -39,6 +39,12 @@ loader.lazyRequireGetter( "resource://devtools/client/definitions.js", true ); +loader.lazyRequireGetter( + this, + "PluralForm", + "resource://devtools/shared/plural-form.js", + true +); const STYLE_INSPECTOR_PROPERTIES = "devtools/shared/locales/styleinspector.properties"; @@ -52,6 +58,7 @@ loader.lazyGetter(this, "NEW_PROPERTY_NAME_INPUT_LABEL", function () { return STYLE_INSPECTOR_L10N.getStr("rule.newPropertyName.label"); }); +const UNUSED_CSS_PROPERTIES_HIDE_THRESHOLD = 10; const INDENT_SIZE = 2; const INDENT_STR = " ".repeat(INDENT_SIZE); @@ -67,6 +74,8 @@ const INDENT_STR = " ".repeat(INDENT_SIZE); * The Rule object we're editing. * @param {Object} options * @param {Set} options.elementsWithPendingClicks + * @param {Function} options.onShowUnusedCustomCssProperties + * @param {Boolean} options.shouldHideUnusedCustomCssProperties */ function RuleEditor(ruleView, rule, options = {}) { EventEmitter.decorate(this); @@ -91,6 +100,8 @@ function RuleEditor(ruleView, rule, options = {}) { this._onToolChanged = this._onToolChanged.bind(this); this._updateLocation = this._updateLocation.bind(this); this._onSourceClick = this._onSourceClick.bind(this); + this._onShowUnusedCustomCssPropertiesButtonClick = + this._onShowUnusedCustomCssPropertiesButtonClick.bind(this); this.rule.domRule.on("location-changed", this._locationChanged); this.toolbox.on("tool-registered", this._onToolChanged); @@ -105,6 +116,12 @@ RuleEditor.prototype = { prop.editor?.destroy(); } + this._unusedCssVariableDeclarations = null; + + if (this._showUnusedCustomCssPropertiesButton) { + this._nullifyShowUnusedCustomCssProperties({ removeFromDom: false }); + } + this.rule.domRule.off("location-changed"); this.toolbox.off("tool-registered", this._onToolChanged); this.toolbox.off("tool-unregistered", this._onToolChanged); @@ -322,11 +339,11 @@ RuleEditor.prototype = { this.ancestorDataEl.append(ancestorsFrag); } - const code = createChild(this.element, "div", { + this.ruleviewCodeEl = createChild(this.element, "div", { class: "ruleview-code", }); - const header = createChild(code, "div", {}); + const header = createChild(this.ruleviewCodeEl, "div", {}); createChild(header, "span", { class: "ruleview-rule-indent", @@ -397,14 +414,14 @@ RuleEditor.prototype = { // We can't use a proper "ol" as it will mess with selection copy text, // adding spaces on list item instead of the one we craft (.ruleview-rule-indent) - this.propertyList = createChild(code, "div", { + this.propertyList = createChild(this.ruleviewCodeEl, "div", { class: "ruleview-propertylist", role: "list", }); this.populate(); - this.closeBrace = createChild(code, "div", { + this.closeBrace = createChild(this.ruleviewCodeEl, "div", { class: "ruleview-ruleclose", tabindex: this.isEditable ? "0" : "-1", }); @@ -425,7 +442,7 @@ RuleEditor.prototype = { } closingBracketsText += "}\n"; } - createChild(code, "div", { + createChild(this.ruleviewCodeEl, "div", { class: "ruleview-ancestor-ruleclose", textContent: closingBracketsText, }); @@ -437,11 +454,11 @@ RuleEditor.prototype = { // check this.ruleview.isEditing on mousedown this._ruleViewIsEditing = false; - code.addEventListener("mousedown", () => { + this.ruleviewCodeEl.addEventListener("mousedown", () => { this._ruleViewIsEditing = this.ruleView.isEditing; }); - code.addEventListener("click", () => { + this.ruleviewCodeEl.addEventListener("click", () => { const selection = this.doc.defaultView.getSelection(); if (selection.isCollapsed && !this._ruleViewIsEditing) { this.newProperty(); @@ -662,7 +679,27 @@ RuleEditor.prototype = { this.propertyList.replaceChildren(); } + this._unusedCssVariableDeclarations = + this._getUnusedCssVariableDeclarations(); + const hideUnusedCssVariableDeclarations = + this._unusedCssVariableDeclarations.size >= + // If the button was already displayed, hide unused variables if we have at least + // one, even if it's less than the threshold + (this._showUnusedCustomCssPropertiesButton + ? 1 + : UNUSED_CSS_PROPERTIES_HIDE_THRESHOLD); + + // If we won't hide any variable, clear the Set of unused variables as it's used in + // updateUnusedCssVariables and we might do unnecessary computation if we still + // track variables which are actually visible. + if (!hideUnusedCssVariableDeclarations) { + this._unusedCssVariableDeclarations.clear(); + } + for (const prop of this.rule.textProps) { + if (hideUnusedCssVariableDeclarations && prop.isUnusedVariable) { + continue; + } if (!prop.editor && !prop.invisible) { const editor = new TextPropertyEditor(this, prop, { elementsWithPendingClicks: this.options.elementsWithPendingClicks, @@ -675,6 +712,29 @@ RuleEditor.prototype = { } } + if (hideUnusedCssVariableDeclarations) { + if (!this._showUnusedCustomCssPropertiesButton) { + this._showUnusedCustomCssPropertiesButton = + this.doc.createElement("button"); + this._showUnusedCustomCssPropertiesButton.classList.add( + "devtools-button", + "devtools-button-standalone", + "ruleview-show-unused-custom-css-properties" + ); + this._showUnusedCustomCssPropertiesButton.addEventListener( + "click", + this._onShowUnusedCustomCssPropertiesButtonClick + ); + } + this.ruleviewCodeEl.insertBefore( + this._showUnusedCustomCssPropertiesButton, + this.closeBrace + ); + this._updateShowUnusedCustomCssPropertiesButtonText(); + } else if (this._showUnusedCustomCssPropertiesButton) { + this._nullifyShowUnusedCustomCssProperties(); + } + // Set focus if the focus is still in the current document (avoid stealing // the focus, see Bug 1911627). if (this.doc.hasFocus() && focusedElSelector) { @@ -689,6 +749,175 @@ RuleEditor.prototype = { } }, + updateUnusedCssVariables() { + if ( + !this._unusedCssVariableDeclarations || + !this._unusedCssVariableDeclarations.size + ) { + return; + } + + // Store the list of what used to be unused + const previouslyUnused = Array.from(this._unusedCssVariableDeclarations); + // Then compute the list of unused variables again + this._unusedCssVariableDeclarations = + this._getUnusedCssVariableDeclarations(); + + for (const prop of previouslyUnused) { + if (this._unusedCssVariableDeclarations.has(prop)) { + continue; + } + + // The prop wasn't used, but now is, so let's show it + this.showUnusedCssVariable(prop, { + updateButton: false, + }); + } + + this._updateShowUnusedCustomCssPropertiesButtonText(); + }, + + /** + * Create a TextPropertyEditor for TextProperty representing an unused CSS variable. + * + * @param {TextProperty} prop + * @param {object} options + * @param {boolean} options.updateButton + * @returns {TextPropertyEditor|null} Returns null if passed TextProperty isn't found + * in the list of unused css variables + */ + showUnusedCssVariable(prop, { updateButton = true } = {}) { + if (prop.editor) { + return null; + } + + this._unusedCssVariableDeclarations.delete(prop); + + const editor = new TextPropertyEditor(this, prop, { + elementsWithPendingClicks: this.options.elementsWithPendingClicks, + }); + const declarationIndex = this.rule.textProps.indexOf(prop); + // We need to insert the editor according to its index in the list of declarations. + // So let's try to find the prop which is placed higher and is visible + let nextSibling; + for (let i = declarationIndex + 1; i < this.rule.textProps.length; i++) { + const currentProp = this.rule.textProps[i]; + if (currentProp.editor) { + nextSibling = currentProp.editor.element; + break; + } + } + // If we couldn't find nextSibling, that means that no declaration with higher index + // is visible, so we can put the newly visible property at the end + this.propertyList.insertBefore(editor.element, nextSibling || null); + + if (updateButton) { + this._updateShowUnusedCustomCssPropertiesButtonText(); + } + + return editor; + }, + + /** + * Returns a Set containing the list of unused CSS variable TextProperty which shouldn't + * be visible. + * + * @returns {Set<TextProperty>} + */ + _getUnusedCssVariableDeclarations() { + const unusedCssVariableDeclarations = new Set(); + + // No need to go through the declarations if we shouldn't hide unused custom properties + if (!this.options.shouldHideUnusedCustomCssProperties) { + return unusedCssVariableDeclarations; + } + + // Compute a list of variables that will be visible, as there might be unused variables + // that will be visible (e.g. if the user added one in the rules view) + for (const prop of this.rule.textProps) { + if (prop.isUnusedVariable) { + unusedCssVariableDeclarations.add(prop); + } + } + + return unusedCssVariableDeclarations; + }, + + /** + * Handle click on "Show X unused custom CSS properties" button + * + * @param {Event} e + */ + _onShowUnusedCustomCssPropertiesButtonClick(e) { + e.stopPropagation(); + + this._nullifyShowUnusedCustomCssProperties(); + + for (const prop of this._unusedCssVariableDeclarations) { + if (!prop.invisible) { + const editor = new TextPropertyEditor(this, prop, { + elementsWithPendingClicks: this.options.elementsWithPendingClicks, + }); + // Insert at the original declaration index + this.propertyList.insertBefore( + editor.element, + this.propertyList.childNodes[this.rule.textProps.indexOf(prop)] || + null + ); + } + } + if (typeof this.options.onShowUnusedCustomCssProperties === "function") { + this.options.onShowUnusedCustomCssProperties(); + } + }, + + /** + * Update the text for the "Show X unused custom CSS properties" button, or remove it + * if there's no hidden custom properties anymore + */ + _updateShowUnusedCustomCssPropertiesButtonText() { + if (!this._showUnusedCustomCssPropertiesButton) { + return; + } + + const unusedVariablesCount = this._unusedCssVariableDeclarations.size; + if (!unusedVariablesCount) { + this._nullifyShowUnusedCustomCssProperties(); + return; + } + + const label = PluralForm.get( + unusedVariablesCount, + STYLE_INSPECTOR_L10N.getStr("rule.showUnusedCssVariable") + ).replace("#1", unusedVariablesCount); + + this._showUnusedCustomCssPropertiesButton.replaceChildren(label); + }, + + /** + * Nullify this._showUnusedCustomCssPropertiesButton, remove its click event handler + * and remove it from the DOM if `removeFromDom` is set to true. + * + * @param {object} [options] + * @param {boolean} [options.removeFromDom] + * Should the button be removed from the DOM (defaults to true) + */ + _nullifyShowUnusedCustomCssProperties({ removeFromDom = true } = {}) { + if (!this._showUnusedCustomCssPropertiesButton) { + return; + } + + this._showUnusedCustomCssPropertiesButton.removeEventListener( + "click", + this._onShowUnusedCustomCssPropertiesButtonClick + ); + + if (removeFromDom) { + this._showUnusedCustomCssPropertiesButton.remove(); + } + this._showUnusedCustomCssPropertiesButton = null; + }, + /** * Render a given rule selector in this.selectorText element * @@ -868,10 +1097,15 @@ RuleEditor.prototype = { return; } - // While we're editing a new property, it doesn't make sense to - // start a second new property editor, so disable focusing the - // close brace for now. + // While we're editing a new property, it doesn't make sense to start a second new + // property editor, so disable focusing the close brace for now. this.closeBrace.removeAttribute("tabindex"); + // We also need to make the "Show Unused Variables" button non-focusable so hitting + // Tab while focused in the new property editor will move the focus to the next rule + // selector editor. + if (this._showUnusedCustomCssPropertiesButton) { + this._showUnusedCustomCssPropertiesButton.setAttribute("tabindex", "-1"); + } this.newPropItem = createChild(this.propertyList, "div", { class: "ruleview-property ruleview-newproperty", @@ -944,8 +1178,11 @@ RuleEditor.prototype = { * event has been fired to keep consistent UI state. */ _newPropertyDestroy() { - // We're done, make the close brace focusable again. + // We're done, make the close brace and "Show unused variable" button focusable again. this.closeBrace.setAttribute("tabindex", "0"); + if (this._showUnusedCustomCssPropertiesButton) { + this._showUnusedCustomCssPropertiesButton.removeAttribute("tabindex"); + } this.propertyList.removeChild(this.newPropItem); delete this.newPropItem; diff --git a/devtools/client/themes/rules.css b/devtools/client/themes/rules.css @@ -946,3 +946,14 @@ .ruleview-propertyvalue-break-spaces { white-space: break-spaces; } + +.ruleview-show-unused-custom-css-properties { + margin-block: 4px; + /* Align the button with the properties above it. */ + /* prettier-ignore */ + margin-inline-start: calc( + var(--rule-enableproperty-size) + + var(--rule-enableproperty-margin-inline-start) + + var(--rule-enableproperty-margin-inline-end) + ); +} diff --git a/devtools/shared/locales/en-US/styleinspector.properties b/devtools/shared/locales/en-US/styleinspector.properties @@ -80,6 +80,14 @@ rule.filterProperty.title=Filter rules containing this property # rule view. rule.variableJumpDefinition.title=Jump to variable definition +# LOCALIZATION NOTE (rule.showUnusedCssVariable): Text for the button displayed in the rules +# view when some of the CSS variables of the rule are hidden. +# This is a semi-colon list of plural forms. +# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals +# #1 is the number of hidden unused variables +# example: Show 345 unused custom CSS properties. +rule.showUnusedCssVariable=Show #1 unused custom CSS property;Show #1 unused custom CSS properties + # LOCALIZATION NOTE (rule.empty): Text displayed when the highlighter is # first opened and there's no node selected in the rule view. rule.empty=No element selected.