1719461 - Don't display unused CSS variables

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, enhancement)

Description

[Nicolas Chevobbe [:nchevobbe]]

Safari recently implemented a change where unused CSS variable are not displayed, but can be by clicking on a button (see screenshot in https://twitter.com/razvancaliman/status/1412733122291154947)
This is something quite interesting as it would reduce clutter in the inspector and might also avoid performance issues we're seeing in some pages which are heavily using CSS properties.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: WIP: Bug 1719461 - [devtools] Compute used variables in parseDeclarations. r=#devtools

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1719461 - [devtools] Hide CSS variables that are not used in regular properties. r=#devtools.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1719461 - [devtools] Add DAMP test for many unused variables. r=#devtools.

This should help track the effectiveness of hiding unused variables.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1719461 - [devtools] Refactor Rule highlighting functions. r=#devtools.

Extract code to highlight a registered css property into its own function (_maybeHighlightCssRegisteredProperty),
to reduce the complexity of highlightProperty.
Remove _highlightRuleProperty and instead call _highlightMatches with a
TextProperty instead of a TextPropertyEditor.
Change _highlightMatches signature so it takes an object and can consume TextProperty.
Change _highlightComputedProeprty to take a TextProperty instead of a
TextPropertyEditor as well.

Comment 6

[Phabricator Automation]

Comment on attachment 9519728 [details]
Bug 1719461 - [devtools] Add DAMP test for many unused variables. r=#devtools.

Revision D268389 was moved to bug 1994925. Setting attachment 9519728 [details] to obsolete.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6618651b9919
https://hg.mozilla.org/integration/autoland/rev/2b0d755cd6cb
[devtools] Refactor Rule highlighting functions. r=devtools-reviewers,ochameau.
https://github.com/mozilla-firefox/firefox/commit/02dc395a6fa0
https://hg.mozilla.org/integration/autoland/rev/41b8d4d7f123
[devtools] Hide CSS variables that are not used in regular properties. r=devtools-reviewers,jdescottes.

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/2b0d755cd6cb
https://hg.mozilla.org/mozilla-central/rev/41b8d4d7f123

Comment 9

[Sebastian Zartner [:sebo]]

I believe, this is worth to be mentioned in the general and developer release notes.

[Why is this notable]: DevTools feature that hides unnecessary information by default.
[Affects Firefox for Android]: no
[Suggested wording]: Unused custom properties are now hidden by default in the Rule view.
[Links (documentation, blog post, etc)]:
Currently not documented, though should be added to https://firefox-source-docs.mozilla.org/devtools-user/page_inspector/how_to/examine_and_edit_css/index.html. I've just filed bug 1996487 for that.

Sebastian

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Sebastian Zartner [:sebo] from comment #9)

I believe, this is worth to be mentioned in the general and developer release notes.

[Why is this notable]: DevTools feature that hides unnecessary information by default.
[Affects Firefox for Android]: no
[Suggested wording]: Unused custom properties are now hidden by default in the Rule view.
[Links (documentation, blog post, etc)]:

we shared about the feature on our social accounts, e.g. https://mastodon.social/@FirefoxDevTools/115412197312026432

Comment 11

[Dianna Smith [:diannaS]]

Added to the 146 nightly notes
https://www.firefox.com/en-US/firefox/146.0a1/releasenotes/

Comment 12

[Alex Finder| PTO returning Dec. 8th]

(In reply to Pulsebot from comment #7)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6618651b9919
https://hg.mozilla.org/integration/autoland/rev/2b0d755cd6cb
[devtools] Refactor Rule highlighting functions.
r=devtools-reviewers,ochameau.
https://github.com/mozilla-firefox/firefox/commit/02dc395a6fa0
https://hg.mozilla.org/integration/autoland/rev/41b8d4d7f123
[devtools] Hide CSS variables that are not used in regular properties.
r=devtools-reviewers,jdescottes.

Perfherder has detected a devtools performance change from push 41b8d4d7f123bac70f2970a3f3a114ecb3a2f3a3.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
54%	damp custom.inspector.manyunusedcssvariables.selectnode	linux1804-64-shippable-qr	e10s fission stylo webrender	113.37 -> 52.67
31%	damp browser-toolbox.webconsole-ready.DAMP	macosx1470-64-shippable	e10s fission stylo webrender	729.88 -> 501.49
30%	damp browser-toolbox.webconsole-ready.DAMP	macosx1470-64-shippable	e10s fission stylo webrender	742.52 -> 516.26
28%	damp browser-toolbox.webconsole-ready.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	651.16 -> 467.73
24%	damp browser-toolbox.webconsole-ready.DAMP	windows11-64-24h2-shippable	e10s fission stylo webrender	404.81 -> 308.80

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 47230

The following documentation link provides more information about this command.

Comment 14

[Chris Mills [:cmills]]

I've added the feature to the Firefox 146 rel notes on MDN in https://github.com/mdn/content/pull/42063.

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Chris Mills [:cmills] from comment #14)

I've added the feature to the Firefox 146 rel notes on MDN in https://github.com/mdn/content/pull/42063.

Thanks Chris 👍