commit 053240086b07dc2c3d461113c49d5c8bf0165519
parent 6d7307c0cd1e0d7d3904f9a0e7f5103a5187db1e
Author: David Shin <dshin@mozilla.com>
Date: Thu, 9 Oct 2025 10:25:48 +0000
Bug 1882379: Generate warning for useless :scope selectors. r=firefox-style-system-reviewers,webidl,layout-reviewers,smaug,emilio
Differential Revision: https://phabricator.services.mozilla.com/D268045
Diffstat:
4 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/dom/webidl/CSSStyleRule.webidl b/dom/webidl/CSSStyleRule.webidl
@@ -38,6 +38,7 @@ interface CSSStyleRule : CSSGroupingRule {
enum SelectorWarningKind {
"UnconstrainedHas",
+ "SiblingCombinatorAfterScope",
};
dictionary SelectorWarning {
diff --git a/layout/inspector/tests/chrome/test_getSelectorWarnings.html b/layout/inspector/tests/chrome/test_getSelectorWarnings.html
@@ -9,6 +9,10 @@ function unconstrainedHas(index) {
return { index: index, kind: "UnconstrainedHas" };
}
+function siblingCombinatorAfterScope(index) {
+ return { index: index, kind: "SiblingCombinatorAfterScope" };
+}
+
function testRule(rule, expected) {
const index = sheet.insertRule(rule);
const warnings = sheet.rules[index].getSelectorWarnings();
@@ -40,4 +44,13 @@ testRule(".bar:has(:is(* .bar) .foo), .bar:has(:is(:where(* .baz) .bar) .foo) {}
testRule(".bar:has(:is(*, .bar) .foo) {}", [unconstrainedHas(0)]);
testRule(":is(:has(*) .bar):has(.foo) {}", [unconstrainedHas(0)]);
testRule(":is(:has(*)):has(.foo) {}", [unconstrainedHas(0)]);
+// `:scope` selector with sibling
+testRule(":scope ~ .foo {}", [siblingCombinatorAfterScope(0)]);
+testRule(":scope ~ .foo, :scope + .foo {}", [siblingCombinatorAfterScope(0), siblingCombinatorAfterScope(1)]);
+testRule(":scope .foo, :scope ~ .foo {}", [siblingCombinatorAfterScope(1)]);
+testRule(":is(.foo :scope) ~ .bar, :is(.foo :scope) + .bar {}", [siblingCombinatorAfterScope(0), siblingCombinatorAfterScope(1)]);
+testRule(":is(:scope .foo) ~ .bar, :is(:scope > .foo) ~ .bar {}", []);
+testRule(":is(.foo :is(.bar :is(.baz :scope))) ~ .baa {}", [siblingCombinatorAfterScope(0)]);
+testRule(".foo:has(:scope ~ .bar) {}", []);
+testRule(".foo:has(:scope) ~ .bar {}", []);
</script>
diff --git a/layout/style/CSSStyleRule.cpp b/layout/style/CSSStyleRule.cpp
@@ -350,6 +350,8 @@ SelectorWarningKind ToWebIDLSelectorWarningKind(
switch (aKind) {
case StyleSelectorWarningKind::UnconstraintedRelativeSelector:
return SelectorWarningKind::UnconstrainedHas;
+ case StyleSelectorWarningKind::SiblingCombinatorAfterScopeSelector:
+ return SelectorWarningKind::SiblingCombinatorAfterScope;
}
MOZ_ASSERT_UNREACHABLE("Unhandled selector warning kind");
// Return something for assert-disabled builds.
diff --git a/servo/components/style/error_reporting.rs b/servo/components/style/error_reporting.rs
@@ -9,7 +9,7 @@
use crate::selector_parser::SelectorImpl;
use crate::stylesheets::UrlExtraData;
use cssparser::{BasicParseErrorKind, ParseErrorKind, SourceLocation, Token};
-use selectors::parser::{Component, RelativeSelector, Selector};
+use selectors::parser::{Combinator, Component, RelativeSelector, Selector};
use selectors::visitor::{SelectorListKind, SelectorVisitor};
use selectors::SelectorList;
use std::fmt;
@@ -282,6 +282,9 @@ pub enum SelectorWarningKind {
/// Relative Selector with not enough constraint, either outside or inside the selector. e.g. `*:has(.a)`, `.a:has(*)`.
/// May cause expensive invalidations for every element inserted and/or removed.
UnconstraintedRelativeSelector,
+ /// `:scope` can have 3 meanings, but in all cases, the relationship is defined strictly by an ancestor-descendant
+ /// relationship. This means that any presence of sibling selectors to its right would make it never match.
+ SiblingCombinatorAfterScopeSelector,
}
impl SelectorWarningKind {
@@ -291,6 +294,9 @@ impl SelectorWarningKind {
if UnconstrainedRelativeSelectorVisitor::has_warning(selector, 0, false) {
result.push(SelectorWarningKind::UnconstraintedRelativeSelector);
}
+ if SiblingCombinatorAfterScopeSelectorVisitor::has_warning(selector) {
+ result.push(SelectorWarningKind::SiblingCombinatorAfterScopeSelector);
+ }
result
}
}
@@ -446,3 +452,67 @@ impl SelectorVisitor for UnconstrainedRelativeSelectorVisitor {
true
}
}
+
+struct SiblingCombinatorAfterScopeSelectorVisitor {
+ right_combinator_is_sibling: bool,
+ found: bool,
+}
+
+impl SiblingCombinatorAfterScopeSelectorVisitor {
+ fn new(right_combinator_is_sibling: bool) -> Self {
+ Self {
+ right_combinator_is_sibling,
+ found: false,
+ }
+ }
+ fn has_warning(selector: &Selector<SelectorImpl>) -> bool {
+ if !selector.has_scope_selector() {
+ return false;
+ }
+ let visitor = SiblingCombinatorAfterScopeSelectorVisitor::new(false);
+ visitor.find_never_matching_scope_selector(selector)
+ }
+
+ fn find_never_matching_scope_selector(mut self, selector: &Selector<SelectorImpl>) -> bool {
+ selector.visit(&mut self);
+ self.found
+ }
+}
+
+impl SelectorVisitor for SiblingCombinatorAfterScopeSelectorVisitor {
+ type Impl = SelectorImpl;
+
+ fn visit_simple_selector(&mut self, c: &Component<Self::Impl>) -> bool {
+ if !matches!(c, Component::Scope | Component::ImplicitScope) {
+ return true;
+ }
+ // e.g. `:scope ~ .a` will never match.
+ if self.right_combinator_is_sibling {
+ self.found = true;
+ }
+ true
+ }
+
+ fn visit_selector_list(
+ &mut self,
+ _list_kind: SelectorListKind,
+ list: &[Selector<Self::Impl>],
+ ) -> bool {
+ for s in list {
+ let list_visitor = Self::new(self.right_combinator_is_sibling);
+ self.found |= list_visitor.find_never_matching_scope_selector(s);
+ }
+ true
+ }
+
+ fn visit_complex_selector(&mut self, combinator_to_right: Option<Combinator>) -> bool {
+ if let Some(c) = combinator_to_right {
+ // Subject compounds' state is determined by the outer visitor. e.g: When there's `:is(.a .b) ~ .c`,
+ // the inner visitor is assumed to be constructed with right_combinator_is_sibling == true.
+ self.right_combinator_is_sibling = c.is_sibling();
+ }
+ true
+ }
+
+ // It's harder to discern if use of :scope <sibling-combinator> is invalid - at least for now, defer.
+}
