XLinfini/sousa-gecko
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 2066283 - Fix handling of passed_rightmost_selector. r=firefox-style-system-reviewers,dshin

Browse Source 
This was uncovered by the discussion in D317358.

The fact that we reset this flag for nested lists to false is wrong.

E.g. we end up treating for `:is(#foo) span` differently to `#foo span`:
the first one would end up with "foo" in mapped_ids, while the second
wouldn't (and would end up being a revalidation selector instead).

I think the `:is(#foo)` behavior ends up being better (it avoids sharing
between two elements with the same id if that id is referenced from
selectors, but in exchange it does a lot less revalidation of children).

This matches how we treat classes, for that matter.

We could remove the flag in this patch, but since we want to use it in
D321095, I'm keeping it. Luckily it seems rustc isn't smart enough
to -Werror about it.

Differential Revision: https://phabricator.services.mozilla.com/D321094
This commit is contained in:
Emilio Cobos Álvarez
2026-08-25 14:52:12 +00:00
committed by ealvarez@mozilla.com
parent 79a6c4aa9f
commit 5666c13937
1 changed files with 7 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
servo/components/style/stylist.rs
+7 -34
View File
@@ -2566,8 +2566,7 @@ struct StylistSelectorVisitor<'a> {
/// inside of, if any
in_selector_list_of: SelectorListKind,
/// The filter with all the id's getting referenced from rightmost
/// selectors.
/// The filter with all the id's getting referenced from selectors.
mapped_ids: &'a mut PrecomputedHashSet<Atom>,
/// The filter with the IDs getting referenced from the selector list of
@@ -2602,19 +2601,8 @@ struct StylistSelectorVisitor<'a> {
document_state_dependencies: &'a mut DocumentState,
}
fn component_needs_revalidation(
c: &Component<SelectorImpl>,
passed_rightmost_selector: bool,
) -> bool {
fn component_needs_revalidation(c: &Component<SelectorImpl>) -> bool {
match *c {
Component::ID(_) => {
// TODO(emilio): This could also check that the ID is not already in
// the rule hash. In that case, we could avoid making this a
// revalidation selector too.
//
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1369611
passed_rightmost_selector
},
Component::AttributeInNoNamespaceExists { .. }
| Component::AttributeInNoNamespace { .. }
| Component::AttributeOther(_)
@@ -2636,7 +2624,7 @@ impl<'a> StylistSelectorVisitor<'a> {
let old_passed_rightmost_selector = self.passed_rightmost_selector;
let old_in_selector_list_of = self.in_selector_list_of;
self.passed_rightmost_selector = false;
// NOTE: Not resetting passed_rightmost_selector, intentionally.
self.in_selector_list_of = in_selector_list_of;
let _ret = selector.visit(self);
debug_assert!(_ret, "We never return false");
@@ -2708,8 +2696,7 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
}
fn visit_simple_selector(&mut self, s: &Component<SelectorImpl>) -> bool {
*self.needs_revalidation = *self.needs_revalidation
|| component_needs_revalidation(s, self.passed_rightmost_selector);
*self.needs_revalidation = *self.needs_revalidation || component_needs_revalidation(s);
match *s {
Component::NonTSPseudoClass(NonTSPseudoClass::CustomState(ref name)) => {
@@ -2732,21 +2719,7 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
}
},
Component::ID(ref id) => {
// We want to stop storing mapped ids as soon as we've moved off
// the rightmost ComplexSelector that is not a pseudo-element.
//
// That can be detected by a visit_complex_selector call with a
// combinator other than None and PseudoElement.
//
// Importantly, this call happens before we visit any of the
// simple selectors in that ComplexSelector.
//
// NOTE(emilio): See the comment regarding on when this may
// break in visit_complex_selector.
if !self.passed_rightmost_selector {
self.mapped_ids.insert(id.0.clone());
}
self.mapped_ids.insert(id.0.clone());
if self.in_selector_list_of.relevant_to_nth_of_dependencies() {
self.nth_of_mapped_ids.insert(id.0.clone());
}
@@ -3937,8 +3910,8 @@ impl CascadeData {
)?;
let mut needs_revalidation = false;
let mut visitor = StylistSelectorVisitor {
needs_revalidation: &mut needs_revalidation,
passed_rightmost_selector: false,
needs_revalidation: &mut needs_revalidation,
in_selector_list_of: SelectorListKind::default(),
mapped_ids: &mut self.mapped_ids,
nth_of_mapped_ids: &mut self.nth_of_mapped_ids,
@@ -4475,8 +4448,8 @@ impl CascadeData {
if let Some(cond) = cur_scope.condition.as_ref() {
let mut _unused = false;
let visitor = StylistSelectorVisitor {
needs_revalidation: &mut _unused,
passed_rightmost_selector: true,
needs_revalidation: &mut _unused,
in_selector_list_of: SelectorListKind::default(),
mapped_ids: &mut self.mapped_ids,
nth_of_mapped_ids: &mut self.nth_of_mapped_ids,