From 272d9758d7c4c07e2409d5e0ac7d5c152f49e7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 22 May 2019 13:28:01 +0000 Subject: [PATCH] style: Fix nested-pseudo-elements matching author rules better. This makes *|*::marker do the intended thing in UA sheets, so I think it's better, and it's a bit less special-casey. We may want to re-introduce the changes for pseudo-elements at some point, depending on the WG decision, but this patch makes each_non_document_style_data consistent with the rule collector. The changes of each_non_document_style_data on their own should fix some bugs, but it doesn't because: * This is only hit for pseudos that allow user-action-state pseudo-classes. * The containing shadow check worked for them anyway. * We don't allow any pseudo after ::slotted() or that isn't tree-abiding per the CSS specs (we should maybe enable one of the moz-range stuff to be tree-abiding). So ::placeholder is the only one that right now fits the bill to trigger the bugs this would fix, but it doesn't since I couldn't make ::placeholder match :hover / :active / :focus anyhow (inside or outside a shadow tree). I've left the ProbeMarkerPseudoStyle changes for now since they are technically a bit more consistent than what was there before, but we could revert those if they cause trouble, we could rely on the UA rule matching, but we would need to change that if we allow the ::foo::before and such to match. Differential Revision: https://phabricator.services.mozilla.com/D29542 --- components/style/dom.rs | 13 +++++++++---- components/style/gecko/wrapper.rs | 2 +- components/style/rule_collector.rs | 16 ++++++++++++---- components/style/stylist.rs | 4 ++++ 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 9a81e2ec774..633dce83655 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -801,24 +801,29 @@ pub trait TElement: { use rule_collector::containing_shadow_ignoring_svg_use; - let mut doc_rules_apply = self.matches_user_and_author_rules(); + let target = self.rule_hash_target(); + if !target.matches_user_and_author_rules() { + return false; + } + + let mut doc_rules_apply = true; // Use the same rules to look for the containing host as we do for rule // collection. - if let Some(shadow) = containing_shadow_ignoring_svg_use(*self) { + if let Some(shadow) = containing_shadow_ignoring_svg_use(target) { doc_rules_apply = false; if let Some(data) = shadow.style_data() { f(data, shadow.host()); } } - if let Some(shadow) = self.shadow_root() { + if let Some(shadow) = target.shadow_root() { if let Some(data) = shadow.style_data() { f(data, shadow.host()); } } - let mut current = self.assigned_slot(); + let mut current = target.assigned_slot(); while let Some(slot) = current { // Slots can only have assigned nodes when in a shadow tree. let shadow = slot.containing_shadow().unwrap(); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 9d987c56308..cec55130c4a 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1405,7 +1405,7 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn matches_user_and_author_rules(&self) -> bool { - !self.rule_hash_target().is_in_native_anonymous_subtree() + !self.is_in_native_anonymous_subtree() } #[inline] diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index e87a86c5e47..2e8ffe5d858 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -13,7 +13,7 @@ use crate::selector_parser::PseudoElement; use crate::shared_lock::Locked; use crate::stylesheets::Origin; use crate::stylist::{AuthorStylesEnabled, Rule, RuleInclusion, Stylist}; -use selectors::matching::{ElementSelectorFlags, MatchingContext}; +use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; use servo_arc::ArcBorrow; use smallvec::SmallVec; @@ -97,8 +97,16 @@ where context: &'a mut MatchingContext<'b, E::Impl>, flags_setter: &'a mut F, ) -> Self { - let rule_hash_target = element.rule_hash_target(); - let matches_user_and_author_rules = element.matches_user_and_author_rules(); + // When we're matching with matching_mode = + // `ForStatelessPseudoeElement`, the "target" for the rule hash is the + // element itself, since it's what's generating the pseudo-element. + let rule_hash_target = match context.matching_mode() { + MatchingMode::ForStatelessPseudoElement => element, + MatchingMode::Normal => element.rule_hash_target(), + }; + + let matches_user_and_author_rules = + rule_hash_target.matches_user_and_author_rules(); // Gecko definitely has pseudo-elements with style attributes, like // ::-moz-color-swatch. @@ -120,8 +128,8 @@ where context, flags_setter, rules, - matches_user_and_author_rules, shadow_cascade_order: 0, + matches_user_and_author_rules, matches_document_author_rules: matches_user_and_author_rules, } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 2953e3f0671..cba1cbac524 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1195,6 +1195,10 @@ impl Stylist { // See [3] for the bug to implement whatever gets resolved, and related // bugs for a bit more context. // + // FIXME(emilio): This should probably work for pseudo-elements (i.e., + // use rule_hash_target().shadow_root() instead of + // element.shadow_root()). + // // [1]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/ // core/css/resolver/style_resolver.cc?l=1267&rcl=90f9f8680ebb4a87d177f3b0833372ae4e0c88d8 // [2]: https://github.com/w3c/csswg-drafts/issues/1995