style: Cleanup selector-matching for nested pseudo-elements, match ::slotted correctly when there's no selector before it, and add tests.

D29542 fixed the bogus checks that was making nested pseudo-elements match
author rules. This adds tests and ends up being just a cleanup, though as it
turns out we it also fixes an issue with ::slotted() matched from
Element.matches.

Differential Revision: https://phabricator.services.mozilla.com/D27529
This commit is contained in:
Emilio Cobos Álvarez 2019-05-24 01:09:15 +00:00
parent 272d9758d7
commit 43444db8a8
9 changed files with 35 additions and 38 deletions

View file

@ -84,12 +84,6 @@ impl<Impl: SelectorImpl> SelectorBuilder<Impl> {
self.current_len = 0;
}
/// Returns true if no simple selectors have ever been pushed to this builder.
#[inline(always)]
pub fn is_empty(&self) -> bool {
self.simple_selectors.is_empty()
}
/// Returns true if combinators have ever been pushed to this builder.
#[inline(always)]
pub fn has_combinators(&self) -> bool {

View file

@ -331,11 +331,9 @@ where
return false;
}
// Advance to the non-pseudo-element part of the selector, but let the
// context note that .
if iter.next_sequence().is_none() {
return true;
}
// Advance to the non-pseudo-element part of the selector.
let next_sequence = iter.next_sequence().unwrap();
debug_assert_eq!(next_sequence, Combinator::PseudoElement);
}
let result =
@ -452,10 +450,6 @@ where
},
Combinator::Part => element.containing_shadow_host(),
Combinator::SlotAssignment => {
debug_assert!(
context.current_host.is_some(),
"Should not be trying to match slotted rules in a non-shadow-tree context"
);
debug_assert!(element
.assigned_slot()
.map_or(true, |s| s.is_html_slot_element()));
@ -677,7 +671,6 @@ where
Component::Slotted(ref selector) => {
// <slots> are never flattened tree slottables.
!element.is_html_slot_element() &&
element.assigned_slot().is_some() &&
context.shared.nest(|context| {
matches_complex_selector(selector.iter(), element, context, flags_setter)
})

View file

@ -2002,9 +2002,7 @@ where
},
SimpleSelectorParseResult::SlottedPseudo(selector) => {
state.insert(SelectorParsingState::AFTER_SLOTTED);
if !builder.is_empty() {
builder.push_combinator(Combinator::SlotAssignment);
}
builder.push_simple_selector(Component::Slotted(selector));
},
SimpleSelectorParseResult::PseudoElement(p) => {
@ -2012,9 +2010,7 @@ where
if !p.accepts_state_pseudo_classes() {
state.insert(SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT);
}
if !builder.is_empty() {
builder.push_combinator(Combinator::PseudoElement);
}
builder.push_simple_selector(Component::PseudoElement(p));
},
}
@ -2828,7 +2824,10 @@ pub mod tests {
assert_eq!(
parse("::before"),
Ok(SelectorList::from_vec(vec![Selector::from_vec(
vec![Component::PseudoElement(PseudoElement::Before)],
vec![
Component::Combinator(Combinator::PseudoElement),
Component::PseudoElement(PseudoElement::Before),
],
specificity(0, 0, 1) | HAS_PSEUDO_BIT,
)]))
);
@ -2836,6 +2835,7 @@ pub mod tests {
parse("::before:hover"),
Ok(SelectorList::from_vec(vec![Selector::from_vec(
vec![
Component::Combinator(Combinator::PseudoElement),
Component::PseudoElement(PseudoElement::Before),
Component::NonTSPseudoClass(PseudoClass::Hover),
],
@ -2846,6 +2846,7 @@ pub mod tests {
parse("::before:hover:hover"),
Ok(SelectorList::from_vec(vec![Selector::from_vec(
vec![
Component::Combinator(Combinator::PseudoElement),
Component::PseudoElement(PseudoElement::Before),
Component::NonTSPseudoClass(PseudoClass::Hover),
Component::NonTSPseudoClass(PseudoClass::Hover),
@ -2958,6 +2959,7 @@ pub mod tests {
specificity(0, 0, 0),
)]))
);
assert_eq!(
parse_ns(":not(svg|*)", &parser),
Ok(SelectorList::from_vec(vec![Selector::from_vec(
@ -3032,6 +3034,8 @@ pub mod tests {
Some(&Component::PseudoElement(PseudoElement::Before))
);
assert_eq!(iter.next(), None);
assert_eq!(iter.next_sequence(), Some(Combinator::PseudoElement));
assert_eq!(iter.next(), None);
assert_eq!(iter.next_sequence(), None);
}

View file

@ -47,9 +47,13 @@ pub trait Element: Sized + Clone + Debug {
///
/// This is guaranteed to be called in a pseudo-element.
fn pseudo_element_originating_element(&self) -> Option<Self> {
debug_assert!(self.is_pseudo_element());
self.parent_element()
}
/// Whether we're matching on a pseudo-element.
fn is_pseudo_element(&self) -> bool;
/// Skips non-element nodes
fn prev_sibling_element(&self) -> Option<Self>;

View file

@ -779,7 +779,7 @@ pub trait TElement:
/// element-backed pseudo-element, in which case we return the originating
/// element.
fn rule_hash_target(&self) -> Self {
if self.implemented_pseudo_element().is_some() {
if self.is_pseudo_element() {
self.pseudo_element_originating_element()
.expect("Trying to collect rules for a detached pseudo-element")
} else {

View file

@ -1098,7 +1098,7 @@ impl<'le> TElement for GeckoElement<'le> {
type TraversalChildrenIterator = GeckoChildrenIterator<'le>;
fn inheritance_parent(&self) -> Option<Self> {
if self.implemented_pseudo_element().is_some() {
if self.is_pseudo_element() {
return self.pseudo_element_originating_element();
}
@ -1471,7 +1471,7 @@ impl<'le> TElement for GeckoElement<'le> {
#[inline]
fn skip_item_display_fixup(&self) -> bool {
debug_assert!(
self.implemented_pseudo_element().is_none(),
!self.is_pseudo_element(),
"Just don't call me if I'm a pseudo, you should know the answer already"
);
self.is_root_of_native_anonymous_subtree()
@ -1918,9 +1918,14 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
Some(shadow.host())
}
#[inline]
fn is_pseudo_element(&self) -> bool {
self.implemented_pseudo_element().is_some()
}
#[inline]
fn pseudo_element_originating_element(&self) -> Option<Self> {
debug_assert!(self.implemented_pseudo_element().is_some());
debug_assert!(self.is_pseudo_element());
let parent = self.closest_anon_subtree_root_parent()?;
// FIXME(emilio): Special-case for <input type="number">s

View file

@ -366,6 +366,10 @@ where
self.element.is_root()
}
fn is_pseudo_element(&self) -> bool {
self.element.is_pseudo_element()
}
fn pseudo_element_originating_element(&self) -> Option<Self> {
self.element
.pseudo_element_originating_element()

View file

@ -734,10 +734,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
E: TElement,
{
if cfg!(debug_assertions) {
if element
.and_then(|e| e.implemented_pseudo_element())
.is_some()
{
if element.map_or(false, |e| e.is_pseudo_element()) {
// It'd be nice to assert `self.style.pseudo == Some(&pseudo)`,
// but we do resolve ::-moz-list pseudos on ::before / ::after
// content, sigh.

View file

@ -233,7 +233,7 @@ where
let mut pseudo_styles = EagerPseudoStyles::default();
if self.element.implemented_pseudo_element().is_none() {
if !self.element.is_pseudo_element() {
let layout_parent_style_for_pseudo = if primary_style.style().is_display_contents() {
layout_parent_style
} else {
@ -293,10 +293,6 @@ where
layout_parent_style: Option<&ComputedValues>,
pseudo: Option<&PseudoElement>,
) -> ResolvedStyle {
debug_assert!(
self.element.implemented_pseudo_element().is_none() || pseudo.is_none(),
"Pseudo-elements can't have other pseudos!"
);
debug_assert!(pseudo.map_or(true, |p| p.is_eager()));
let implemented_pseudo = self.element.implemented_pseudo_element();
@ -477,8 +473,8 @@ where
);
debug_assert!(pseudo_element.is_eager());
debug_assert!(
self.element.implemented_pseudo_element().is_none(),
"Element pseudos can't have any other pseudo."
!self.element.is_pseudo_element(),
"Element pseudos can't have any other eager pseudo."
);
let mut applicable_declarations = ApplicableDeclarationList::new();