1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Bug 1440877 found a case where we were defining a variable (--toolbar-gbimage) but never referenced it. Whether it is a result of a typo or unnecessary CSS, we can alert developers when they are adding effectively dead code to the tree.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review commit: https://reviewboard.mozilla.org/r/223906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223906/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=32ee61c46960e684af75ec7b18ee8c1fe3bdd21e

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/1-2/

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8979da6315861de1f10b872b5cf0ff0fc9e4d750

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/2-3/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae906932ad4de57292f985f2f7a053e49a08cf25

Comment 7

[:Gijs (he/him)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

https://reviewboard.mozilla.org/r/223906/#review229986

Thanks!

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/3-4/

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/4-5/

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=07e69a9cbba384ce54a15171b78de1542e77c1b5

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/5-6/

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=027582b14bb60822b29ca423683b7f7294629edc

Comment 13

[Tim Nguyen :ntim]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

https://reviewboard.mozilla.org/r/223906/#review230092

::: browser/base/content/test/static/browser_parsable_css.js:137
(Diff revision 6)
> +  {propName: "--in-content-category-background",
> +   isFromDevTools: false},

--in-content-category-background is actually not used.

https://searchfox.org/mozilla-central/search?q=--in-content-category-background&case=true&regexp=false&path=

::: browser/base/content/test/static/browser_parsable_css.js:139
(Diff revision 6)
> +  {propName: "--separator-border-image",
> +   isFromDevTools: true},

This is not counted because the usage of border-image seems wrong.

normally, it should be `border-image: var(--separator-border-image) 1 1;`

Atm, it's used as `border-image: var(--separator-border-image);` where var(--separator-border-image) is a gradient, which is incorrect.

::: browser/base/content/test/static/browser_parsable_css.js:141
(Diff revision 6)
> +  {propName: "--bezier-diagonal-color",
> +   isFromDevTools: true},
> +  {propName: "--bezier-grid-color",
> +   isFromDevTools: true},

These seem used within a gradient, not sure your code handles that atm.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8954791 [details]
Bug 1441882 - Update browser_parsable_css.js to check that variables that are defined are actually referenced.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223906/diff/6-7/

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e05f1b82846
Update browser_parsable_css.js to check that variables that are defined are actually referenced. r=Gijs

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9b33e8d7623
Update browser_parsable_css.js to check that variables that are defined are actually referenced: Remove --breakpoint-hover-background because it has been already fixed. r=bustage-fix CLOSED TREE

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/5e05f1b82846
https://hg.mozilla.org/mozilla-central/rev/e9b33e8d7623