1561001 - [meta] Stop using the preprocessor for CSS

Status: RESOLVED DUPLICATE of bug 1659444

(Toolkit :: Themes, task, P5)

Description

[Brian Grinstead [:bgrins]]

With a combination of @import and CSS variables I think this shouldn't be technically needed anymore. Having the preprocessor directives confuse tooling and make it harder to map line numbers back to rules in devtools and (potentially) CSS coverage tools. Rather than adding source map support to the preprocessor, I think we should work on reducing reliance on the preprocessor here and ultimately removing it.

Here are some searches showing common usage of the preprocessor:

• https://searchfox.org/mozilla-central/search?q=%25include&case=false&regexp=false&path=.css
• https://searchfox.org/mozilla-central/search?q=%40.*%40&regexp=true&path=.css

Comment 1

[Brian Grinstead [:bgrins]]

This came up in the context of potentially adding CSS code coverage to our ccov jobs. We do have a way to work around the preprocessor for JS code coverage, so we could do something similar if needed:

We have a similar situation with JavaScript files. The JS engine is
producing raw files, and we have a "rewriter" tool which fixes paths and
lines according to preprocessor information.
https://searchfox.org/mozilla-central/rev/f8b11433159cbc9cc80500b3e579d767473fa539/python/mozbuild/mozbuild/codecoverage/lcov_rewriter.py
https://searchfox.org/mozilla-central/rev/f8b11433159cbc9cc80500b3e579d767473fa539/python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py

Comment 2

[Brian Grinstead [:bgrins]]

Here are the consumes of the preprocessor in our CSS files: https://searchfox.org/mozilla-central/search?q=%5E%25&regexp=true&path=.css

Comment 3

[Dão Gottwald [:dao]]

CSS variables can add runtime overhead, which is why we prefer defines when the values aren't supposed to change.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Dão Gottwald [::dao] from comment #3)

CSS variables can add runtime overhead, which is why we prefer defines when the values aren't supposed to change.

Sure, there's a trade off here in order to more easily use existing tooling (for things like code coverage and devtools line number mapping). In theory we could add source map exporting to the preprocessor to help, but I'd prefer to rely on existing standards when possible.

Is there a measurable performance impact to using incrementally more variables? It looks like we already have ~75 on the :root element in browser.xhtml and I'm not aware of reports related to this from the perf team. I just did some searching around and the only bugs I found are:

• Bug 1362593 to add a test to measure performance which hasn't had any attention (which is maybe a sign that this just hasn't come up in practice)
• Bug 1417970, but this is related to when properties change, which they wouldn't here

Comment 5

[Brian Grinstead [:bgrins]]

I see this is being picked up in Bug 1659444