Skip to content

Commit ec21cec

Browse files
committed
Bug 1954142 - Fix StyleResolver to account for element-backed pseudo elements. r=keithamus
This allows for proper parsing of nested pseudo elements such as `::before::marker`, `::details-content::before`, `::details-content::first-letter`, and so on. Co-authored-by: Luke Warlow <[email protected]> Co-authored-by: Keith Cirkel <[email protected]> Differential Revision: https://phabricator.services.mozilla.com/D254456
1 parent a1738c7 commit ec21cec

File tree

6 files changed

+122
-73
lines changed

6 files changed

+122
-73
lines changed

selectors/parser.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,32 @@ impl<Impl: SelectorImpl> Selector<Impl> {
938938
None
939939
}
940940

941+
#[inline]
942+
pub fn pseudo_elements(&self) -> SmallVec<[&Impl::PseudoElement; 3]> {
943+
let mut pseudos = SmallVec::new();
944+
945+
if !self.has_pseudo_element() {
946+
return pseudos;
947+
}
948+
949+
let mut iter = self.iter();
950+
loop {
951+
for component in &mut iter {
952+
if let Component::PseudoElement(ref pseudo) = *component {
953+
pseudos.push(pseudo);
954+
}
955+
}
956+
match iter.next_sequence() {
957+
Some(Combinator::PseudoElement) => {},
958+
_ => break,
959+
}
960+
}
961+
962+
debug_assert!(!pseudos.is_empty(), "has_pseudo_element lied!");
963+
964+
pseudos
965+
}
966+
941967
/// Whether this selector (pseudo-element part excluded) matches every element.
942968
///
943969
/// Used for "pre-computed" pseudo-elements in components/style/stylist.rs
@@ -3717,6 +3743,7 @@ pub mod tests {
37173743
Before,
37183744
After,
37193745
Marker,
3746+
DetailsContent,
37203747
Highlight(String),
37213748
}
37223749

@@ -3740,7 +3767,7 @@ pub mod tests {
37403767
}
37413768

37423769
fn is_element_backed(&self) -> bool {
3743-
false
3770+
matches!(self, Self::DetailsContent)
37443771
}
37453772
}
37463773

@@ -3784,6 +3811,7 @@ pub mod tests {
37843811
PseudoElement::Before => dest.write_str("::before"),
37853812
PseudoElement::After => dest.write_str("::after"),
37863813
PseudoElement::Marker => dest.write_str("::marker"),
3814+
PseudoElement::DetailsContent => dest.write_str("::details-content"),
37873815
PseudoElement::Highlight(ref name) => {
37883816
dest.write_str("::highlight(")?;
37893817
serialize_identifier(&name, dest)?;
@@ -3954,6 +3982,7 @@ pub mod tests {
39543982
"before" => return Ok(PseudoElement::Before),
39553983
"after" => return Ok(PseudoElement::After),
39563984
"marker" => return Ok(PseudoElement::Marker),
3985+
"details-content" => return Ok(PseudoElement::DetailsContent),
39573986
_ => {}
39583987
}
39593988
Err(
@@ -4645,6 +4674,26 @@ pub mod tests {
46454674
assert!(parse("::marker::marker").is_err());
46464675
}
46474676

4677+
#[test]
4678+
fn test_pseudo_on_element_backed_pseudo() {
4679+
let list = parse("::details-content::before").unwrap();
4680+
let selector = &list.slice()[0];
4681+
let mut iter = selector.iter();
4682+
assert_eq!(
4683+
iter.next(),
4684+
Some(&Component::PseudoElement(PseudoElement::Before))
4685+
);
4686+
assert_eq!(iter.next(), None);
4687+
let combinator = iter.next_sequence();
4688+
assert_eq!(combinator, Some(Combinator::PseudoElement));
4689+
assert!(matches!(iter.next(), Some(&Component::PseudoElement(PseudoElement::DetailsContent))));
4690+
assert_eq!(iter.next(), None);
4691+
let combinator = iter.next_sequence();
4692+
assert_eq!(combinator, Some(Combinator::PseudoElement));
4693+
assert_eq!(iter.next(), None);
4694+
assert_eq!(iter.next_sequence(), None);
4695+
}
4696+
46484697
#[test]
46494698
fn test_universal() {
46504699
let list = parse_ns(

style/dom.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -751,12 +751,12 @@ pub trait TElement:
751751
/// element-backed pseudo-element, in which case we return the originating
752752
/// element.
753753
fn rule_hash_target(&self) -> Self {
754-
if self.is_pseudo_element() {
755-
self.pseudo_element_originating_element()
754+
let mut cur = *self;
755+
while cur.is_pseudo_element() {
756+
cur = cur.pseudo_element_originating_element()
756757
.expect("Trying to collect rules for a detached pseudo-element")
757-
} else {
758-
*self
759758
}
759+
cur
760760
}
761761

762762
/// Executes the callback for each applicable style rule data which isn't

style/gecko/pseudo_element.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ impl PseudoElementTrait for PseudoElement {
220220
/// flat tree parent, which might not be the originating element.
221221
#[inline]
222222
fn is_element_backed(&self) -> bool {
223-
// Note: We doen't include ::view-transition here because it inherits from the originating
223+
// Note: We don't include ::view-transition here because it inherits from the originating
224224
// element, instead of the snapshot containing block.
225225
self.is_named_view_transition() || *self == PseudoElement::DetailsContent
226226
}

style/rule_collector.rs

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::selector_parser::PseudoElement;
1313
use crate::shared_lock::Locked;
1414
use crate::stylesheets::{layer_rule::LayerOrder, Origin};
1515
use crate::stylist::{AuthorStylesEnabled, CascadeData, Rule, RuleInclusion, Stylist};
16-
use selectors::matching::{MatchingContext, MatchingMode};
16+
use selectors::matching::MatchingContext;
1717
use servo_arc::ArcBorrow;
1818
use smallvec::SmallVec;
1919

@@ -66,7 +66,7 @@ where
6666
element: E,
6767
rule_hash_target: E,
6868
stylist: &'a Stylist,
69-
pseudo_element: Option<&'a PseudoElement>,
69+
pseudo_elements: SmallVec<[PseudoElement; 1]>,
7070
style_attribute: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
7171
smil_override: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
7272
animation_declarations: AnimationDeclarations,
@@ -86,37 +86,30 @@ where
8686
pub fn new(
8787
stylist: &'a Stylist,
8888
element: E,
89-
pseudo_element: Option<&'a PseudoElement>,
89+
pseudo_elements: SmallVec<[PseudoElement; 1]>,
9090
style_attribute: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
9191
smil_override: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
9292
animation_declarations: AnimationDeclarations,
9393
rule_inclusion: RuleInclusion,
9494
rules: &'a mut ApplicableDeclarationList,
9595
context: &'a mut MatchingContext<'b, E::Impl>,
9696
) -> Self {
97-
// When we're matching with matching_mode =
98-
// `ForStatelessPseudoeElement`, the "target" for the rule hash is the
99-
// element itself, since it's what's generating the pseudo-element.
100-
let rule_hash_target = match context.matching_mode() {
101-
MatchingMode::ForStatelessPseudoElement => element,
102-
MatchingMode::Normal => element.rule_hash_target(),
103-
};
104-
97+
let rule_hash_target = element.rule_hash_target();
10598
let matches_user_and_content_rules = rule_hash_target.matches_user_and_content_rules();
10699

107100
// Gecko definitely has pseudo-elements with style attributes, like
108101
// ::-moz-color-swatch.
109102
debug_assert!(
110-
cfg!(feature = "gecko") || style_attribute.is_none() || pseudo_element.is_none(),
103+
cfg!(feature = "gecko") || style_attribute.is_none() || pseudo_elements.is_empty(),
111104
"Style attributes do not apply to pseudo-elements"
112105
);
113-
debug_assert!(pseudo_element.map_or(true, |p| !p.is_precomputed()));
106+
debug_assert!(pseudo_elements.iter().all(|p| !p.is_precomputed()));
114107

115108
Self {
116109
element,
117110
rule_hash_target,
118111
stylist,
119-
pseudo_element,
112+
pseudo_elements,
120113
style_attribute,
121114
smil_override,
122115
animation_declarations,
@@ -162,7 +155,7 @@ where
162155
};
163156

164157
let cascade_data = self.stylist.cascade_data().borrow_for_origin(origin);
165-
let map = match cascade_data.normal_rules(self.pseudo_element) {
158+
let map = match cascade_data.normal_rules(&self.pseudo_elements) {
166159
Some(m) => m,
167160
None => return,
168161
};
@@ -180,7 +173,7 @@ where
180173

181174
#[cfg(feature = "gecko")]
182175
fn collect_view_transition_dynamic_rules(&mut self) {
183-
if !self.pseudo_element.is_some_and(|p| p.is_named_view_transition()) {
176+
if !self.pseudo_elements.first().is_some_and(|p| p.is_named_view_transition()) {
184177
return;
185178
}
186179
let len_before_vt_rules = self.rules.len();
@@ -205,7 +198,7 @@ where
205198
/// These go before author rules, but after user rules, see:
206199
/// https://drafts.csswg.org/css-cascade/#preshint
207200
fn collect_presentational_hints(&mut self) {
208-
if self.pseudo_element.is_some() {
201+
if !self.pseudo_elements.is_empty() {
209202
return;
210203
}
211204

@@ -289,7 +282,7 @@ where
289282
Some(d) => d,
290283
None => continue,
291284
};
292-
let slotted_rules = match data.slotted_rules(self.pseudo_element) {
285+
let slotted_rules = match data.slotted_rules(&self.pseudo_elements) {
293286
Some(r) => r,
294287
None => continue,
295288
};
@@ -323,7 +316,7 @@ where
323316

324317
let cascade_level = CascadeLevel::same_tree_author_normal();
325318
self.in_shadow_tree(containing_shadow.host(), |collector| {
326-
if let Some(map) = cascade_data.normal_rules(collector.pseudo_element) {
319+
if let Some(map) = cascade_data.normal_rules(&collector.pseudo_elements) {
327320
collector.collect_rules_in_map(map, cascade_level, cascade_data);
328321
}
329322

@@ -333,7 +326,7 @@ where
333326
return;
334327
}
335328

336-
let part_rules = match cascade_data.part_rules(collector.pseudo_element) {
329+
let part_rules = match cascade_data.part_rules(&collector.pseudo_elements) {
337330
Some(p) => p,
338331
None => return,
339332
};
@@ -358,7 +351,7 @@ where
358351
None => return,
359352
};
360353

361-
let host_rules = match style_data.featureless_host_rules(self.pseudo_element) {
354+
let host_rules = match style_data.featureless_host_rules(&self.pseudo_elements) {
362355
Some(rules) => rules,
363356
None => return,
364357
};
@@ -415,7 +408,7 @@ where
415408
};
416409

417410
if let Some(cascade_data) = cascade_data {
418-
if let Some(part_rules) = cascade_data.part_rules(self.pseudo_element) {
411+
if let Some(part_rules) = cascade_data.part_rules(&self.pseudo_elements) {
419412
let containing_host = outer_shadow.map(|s| s.host());
420413
let cascade_level = CascadeLevel::AuthorNormal {
421414
shadow_cascade_order,

style/style_resolver.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,7 @@ where
297297

298298
let mut pseudo_styles = EagerPseudoStyles::default();
299299

300-
// FIXME(bug 1954142): This should account for element-backed pseudo elements.
301-
if !self.element.is_pseudo_element() {
300+
if !self.element.implemented_pseudo_element().is_some_and(|p| !PseudoElementTrait::is_element_backed(&p)) {
302301
let layout_parent_style_for_pseudo =
303302
layout_parent_style_for_pseudo(&primary_style, layout_parent_style);
304303
SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
@@ -377,9 +376,6 @@ where
377376
) -> ResolvedStyle {
378377
debug_assert!(pseudo.map_or(true, |p| p.is_eager()));
379378

380-
let implemented_pseudo = self.element.implemented_pseudo_element();
381-
let pseudo = pseudo.or(implemented_pseudo.as_ref());
382-
383379
let mut conditions = Default::default();
384380
let values = self.context.shared.stylist.cascade_style_and_visited(
385381
Some(self.element),
@@ -523,11 +519,10 @@ where
523519
);
524520

525521
let stylist = &self.context.shared.stylist;
526-
let implemented_pseudo = self.element.implemented_pseudo_element();
527522
// Compute the primary rule node.
528523
stylist.push_applicable_declarations(
529524
self.element,
530-
implemented_pseudo.as_ref(),
525+
None,
531526
self.element.style_attribute(),
532527
self.element.smil_override(),
533528
self.element.animation_declarations(self.context.shared),
@@ -571,10 +566,6 @@ where
571566
self.element, pseudo_element, visited_handling
572567
);
573568
debug_assert!(pseudo_element.is_eager());
574-
debug_assert!(
575-
!self.element.is_pseudo_element(),
576-
"Element pseudos can't have any other eager pseudo."
577-
);
578569

579570
let mut applicable_declarations = ApplicableDeclarationList::new();
580571

0 commit comments

Comments
 (0)