1833679 - Generate DevTools warning for Unconstrained `:has()` selectors

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[David Shin[:dshin]]

Unconstrained :has(...) (Unlike say div:has(...)) selectors require us to try matching all elements, and disable style sharing. This has pretty bad perf implications, so we should warn the authors.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This code is related.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

I'm wondering if this should be a CSS warning (i.e. the user needs to enable the CSS filter on the console to see it)
In such case, it might be lost in a see of other warnings
It might be more impactful to show an icon + tooltip in the rule view, where people writing CSS might spend their time in

It makes me wonder if there are other selectors that could have similar impact on performance

Comment 3

[David Shin[:dshin]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)

In such case, it might be lost in a see of other warnings

Unfortunately, yeah...

It might be more impactful to show an icon + tooltip in the rule view, where people writing CSS might spend their time in

Hm - would it be something like a built-in CSS linter? That'd be useful.

It makes me wonder if there are other selectors that could have similar impact on performance

At least I'd say :has has the largest impact by far.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to David Shin[:dshin] from comment #3)

It might be more impactful to show an icon + tooltip in the rule view, where people writing CSS might spend their time in

Hm - would it be something like a built-in CSS linter? That'd be useful.

yes I think it would be nice. I'm currently thinking about this piece of UI in DevTools (with nesting on its way, it might be nice to have a way to display desugared selector and specificity), and so it's a good opportunity to think about this "slow" warning.

Is there a way we could expose a Chrome-only boolean property (e.g. slowSelector) on the CSSRule so DevTools could identify those?
It might even be better to have 1. the selector indexes of those slow selector and 2. some kind of string that would link to a localized error message that DevTools could retrieve

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

David, what do you think of what I suggest in Comment 4 ?
Would that be hard to implement?
Also, could you provide a test page where :has is slow so we can use it while prototyping this?

thanks

Comment 6

[David Shin[:dshin]]

This is doable, and I think it makes a lot of sense. Just need to get the interface right.

I have gotten started on prototyping this, but my focus is on bug 1792501 at the moment.
There are many examples where :has would become slow, but .anchor:has(*) (Which would have to invalidate every time anything inserts/removes in DOM) is a good example. I'll try to assemble a document.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)

Is there a way we could expose a Chrome-only boolean property (e.g. slowSelector) on the CSSRule so DevTools could identify those?
It might even be better to have 1. the selector indexes of those slow selector and 2. some kind of string that would link to a localized error message that DevTools could retrieve

Alternatively, we could have a InspectorUtils method that we would call to get warning messages about a selector text (or the rule directly, depending what would be the most efficient)
So for example InspectorUtils.getSelectorsWarning(selectorText | cssRule) would return an array of objects of the following form:

{
  selectorIndex: 0,
  warningL10N: "unconstrainedHas"
  type: "slow" // not sure about this, but we might get other kind of warnings in the future ? 
}

I feel like this would be simpler to implement than what I suggested in Comment 4 (and wouldn't have performance impact when DevTools is not open).
Let me know what you think David, it would be nice to have this soon since :has is already enabled in Nightly

Comment 8

[David Shin[:dshin]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #7)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)

Is there a way we could expose a Chrome-only boolean property (e.g. slowSelector) on the CSSRule so DevTools could identify those?
It might even be better to have 1. the selector indexes of those slow selector and 2. some kind of string that would link to a localized error message that DevTools could retrieve

Alternatively, we could have a InspectorUtils method that we would call to get warning messages about a selector text (or the rule directly, depending what would be the most efficient)
So for example InspectorUtils.getSelectorsWarning(selectorText | cssRule) would return an array of objects of the following form:

{
  selectorIndex: 0,
  warningL10N: "unconstrainedHas"
  type: "slow" // not sure about this, but we might get other kind of warnings in the future ? 
}

I feel like this would be simpler to implement than what I suggested in Comment 4 (and wouldn't have performance impact when DevTools is not open).
Let me know what you think David, it would be nice to have this soon since :has is already enabled in Nightly

(Tying up loose ends on :has, but getting to this very soon)

Ah, I like that approach, actually - seems to be a more "right" place.
As per bug 1833679 comment 1, we'd have "neverMatches" type as well.

Comment 9

[David Shin[:dshin]]

Attachment: Bug 1833679 - DevTools warnings infrastructure. r=nchevobbe,emilio

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b32fede7af39
DevTools warnings infrastructure. r=nchevobbe,emilio

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b32fede7af39