Bug 1639533 - Fix a no-longer valid assumption in pseudo-element matching / invalidation code. r=heycam

After bug 1632647, we can have pseudo-classes inside :not / :is /
:where, which the invalidation and matching code weren't handling.

Add a few tests for this stuff working as expected.

Differential Revision: https://phabricator.services.mozilla.com/D76160
This commit is contained in:
Emilio Cobos Álvarez 2020-05-20 23:53:34 +00:00
parent a594bf82aa
commit b50a39c2d3
8 changed files with 110 additions and 19 deletions

View File

@ -0,0 +1,15 @@
<!doctype html>
<style>
.tweak ::-moz-range-thumb:is(:hover) {
background: red;
}
</style>
<div>
<input type=range>
</div>
<script>
onload = function() {
document.querySelector("div").getBoundingClientRect();
document.querySelector("div").classList.add("tweak");
}
</script>

View File

@ -319,3 +319,4 @@ load 1599286.html
pref(layout.css.motion-path.enabled,true) load 1609786.html
pref(layout.css.constructable-stylesheets.enabled,true) load 1616433.html
pref(layout.css.constructable-stylesheets.enabled,true) load 1616407.html
load 1639533.html

View File

@ -75,6 +75,24 @@ var gTests = [
active_test_style: 'body input::-moz-range-thumb:active { background: lime; }',
active_reference_style: 'body input::-moz-range-thumb { background: lime; }' },
// Do the same as above, but using :is instead.
{ markup: '<input type="range" value="50" min="0" max="100">',
pseudoelement: '::-moz-range-thumb',
common_style: 'body input { -moz-appearance: none; } input::-moz-range-thumb { background: black; }',
hover_test_style: 'body input::-moz-range-thumb:is(:hover) { background: green; }',
hover_reference_style: 'body input::-moz-range-thumb { background: green; }',
active_test_style: 'body input::-moz-range-thumb:is(:active) { background: lime; }',
active_reference_style: 'body input::-moz-range-thumb { background: lime; }' },
// Do the same as above, but using :not instead.
{ markup: '<input type="range" value="50" min="0" max="100">',
pseudoelement: '::-moz-range-thumb',
common_style: 'input { -moz-appearance: none; } input::-moz-range-thumb { background: green } input::-moz-range-thumb:not(:hover) { background: black; }',
hover_test_style: '',
hover_reference_style: 'input::-moz-range-thumb { background: green !important; }',
active_test_style: 'input::-moz-range-thumb:active { background: lime !important; }',
active_reference_style: 'input::-moz-range-thumb { background: lime !important; }' },
// ::placeholder can't be tested, since the UA style sheet sets it to
// be pointer-events:none.
];

View File

@ -1308,7 +1308,14 @@ function runTests() {
// CSS_PSEUDO_ELEMENT_SUPPORTS_USER_ACTION_STATE (none of which are
// unprefixed).
test_parseable("::-moz-color-swatch:hover");
test_parseable("::-moz-color-swatch:is(:hover)");
test_parseable("::-moz-color-swatch:not(:hover)");
test_parseable("::-moz-color-swatch:where(:hover)");
test_balanced_unparseable("::-moz-color-swatch:not(.foo)");
test_balanced_unparseable("::-moz-color-swatch:where(.foo)");
test_balanced_unparseable("::-moz-color-swatch:is(.foo)");
test_balanced_unparseable("::-moz-color-swatch:where(p, :hover)");
test_balanced_unparseable("::-moz-color-swatch:is(p, :hover)");
test_balanced_unparseable("::-moz-color-swatch:first-child");
test_balanced_unparseable("::-moz-color-swatch:host");
test_balanced_unparseable("::-moz-color-swatch:host(div)");

View File

@ -322,14 +322,13 @@ where
},
}
// The only other parser-allowed Component in this sequence is a state
// class. We just don't match in that case.
if let Some(s) = iter.next() {
debug_assert!(
matches!(*s, Component::NonTSPseudoClass(..)),
"Someone messed up pseudo-element parsing"
);
return false;
for component in &mut iter {
// The only other parser-allowed Components in this sequence are
// state pseudo-classes, or one of the other things that can contain
// them.
if !component.matches_for_stateless_pseudo_element() {
return false;
}
}
// Advance to the non-pseudo-element part of the selector.

View File

@ -1054,6 +1054,47 @@ impl<Impl: SelectorImpl> Component<Impl> {
}
}
/// Whether this component is valid after a pseudo-element. Only intended
/// for sanity-checking.
pub fn maybe_allowed_after_pseudo_element(&self) -> bool {
match *self {
Component::NonTSPseudoClass(..) => true,
Component::Negation(ref components) => components.iter().all(|c| c.maybe_allowed_after_pseudo_element()),
Component::Is(ref selectors) |
Component::Where(ref selectors) => {
selectors.iter().all(|selector| {
selector.iter_raw_match_order().all(|c| c.maybe_allowed_after_pseudo_element())
})
},
_ => false,
}
}
/// Whether a given selector should match for stateless pseudo-elements.
///
/// This is a bit subtle: Only selectors that return true in
/// `maybe_allowed_after_pseudo_element` should end up here, and
/// `NonTSPseudoClass` never matches (as it is a stateless pseudo after
/// all).
pub(crate) fn matches_for_stateless_pseudo_element(&self) -> bool {
debug_assert!(
self.maybe_allowed_after_pseudo_element(),
"Someone messed up pseudo-element parsing: {:?}",
*self
);
match *self {
Component::Negation(ref components) => {
!components.iter().all(|c| c.matches_for_stateless_pseudo_element())
},
Component::Is(ref selectors) | Component::Where(ref selectors) => {
selectors.iter().any(|selector| {
selector.iter_raw_match_order().all(|c| c.matches_for_stateless_pseudo_element())
})
},
_ => false,
}
}
pub fn visit<V>(&self, visitor: &mut V) -> bool
where
V: SelectorVisitor<Impl = Impl>,

View File

@ -871,23 +871,29 @@ where
// This will usually be the very next component, except for
// the fact that we store compound selectors the other way
// around, so there could also be state pseudo-classes.
let pseudo_selector = next_invalidation
let pseudo = next_invalidation
.dependency
.selector
.iter_raw_parse_order_from(next_invalidation.offset)
.skip_while(|c| matches!(**c, Component::NonTSPseudoClass(..)))
.flat_map(|c| {
if let Component::PseudoElement(ref pseudo) = *c {
return Some(pseudo);
}
// TODO: Would be nice to make this a diagnostic_assert! of
// sorts.
debug_assert!(
c.maybe_allowed_after_pseudo_element(),
"Someone seriously messed up selector parsing: \
{:?} at offset {:?}: {:?}",
next_invalidation.dependency, next_invalidation.offset, c,
);
None
})
.next()
.unwrap();
let pseudo = match *pseudo_selector {
Component::PseudoElement(ref pseudo) => pseudo,
_ => unreachable!(
"Someone seriously messed up selector parsing: \
{:?} at offset {:?}: {:?}",
next_invalidation.dependency, next_invalidation.offset, pseudo_selector,
),
};
// FIXME(emilio): This is not ideal, and could not be
// accurate if we ever have stateful element-backed eager
// pseudos.

View File

@ -17,6 +17,10 @@
test_invalid_selector("::slotted(*):host");
test_invalid_selector("::slotted(*):host(div)");
test_invalid_selector("::slotted(*):hover");
test_invalid_selector("::slotted(*):is(:hover)");
test_invalid_selector("::slotted(*):where(:hover)");
test_invalid_selector("::slotted(*):is(#id)");
test_invalid_selector("::slotted(*):where(#id)");
test_invalid_selector("::slotted(*):read-only");
test_invalid_selector("::slotted(*)::slotted(*)");
test_invalid_selector("::slotted(*)::before::slotted(*)");