From 5507d53611d7eb3d7f114a863459f3a8b096b10b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 10 May 2018 18:48:08 +0200 Subject: [PATCH] style: Make element-backed pseudos inherit from NAC subtree roots and other NAC inherit from their parents. Currently, NAC always inherits from the closest non-NAC ancestor element, regardless of whether it is for an element-backed pseudo or not. This patch changes the inheritance so that for element-backed pseudos, we inherit from the closest native anonymous root's parent, and for other NAC we inherit from the parent. This prevents the following two issues and allows us to remove the NODE_IS_NATIVE_ANONYMOUS flag: * Avoiding inheriting from the non-NAC ancestor in XBL bindings bound to NAC. - This is no longer a problem since we apply the rule only if we're a pseudo-element, and all pseudo-elements are in native anonymous subtrees. - This also allows to remove the hack that propagates the NODE_IS_NATIVE_ANONYMOUS flag from the ::cue pseudo-element from BindToTree. * Inheriting from the wrong thing if we're a nested NAC subtree. - We no longer look past our NAC subtree, with the exception of ::-moz-number-text's pseudo-elements, for which we do want to propagate ::placeholder to. A few rules from forms.css have been modified because they're useless or needed to propagate stuff to the anonymous form control in input[type="number"] which previously inherited from the input itself. Bug: 1460382 Reviewed-by: heycam MozReview-Commit-ID: IDKYt3EJtSH --- components/style/dom.rs | 13 ++---- components/style/gecko/wrapper.rs | 70 +++++++++++++++++------------- components/style/matching.rs | 2 +- components/style/sharing/mod.rs | 6 +-- components/style/style_resolver.rs | 4 +- components/style/traversal.rs | 7 +-- 6 files changed, 53 insertions(+), 49 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 0e5da1542d7..42f6e9ba0c0 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -466,12 +466,6 @@ pub trait TElement: &[] } - /// For a given NAC element, return the closest non-NAC ancestor, which is - /// guaranteed to exist. - fn closest_non_native_anonymous_ancestor(&self) -> Option { - unreachable!("Servo doesn't know about NAC"); - } - /// Get this element's style attribute. fn style_attribute(&self) -> Option>>; @@ -657,9 +651,8 @@ pub trait TElement: false } - /// Returns true if this element is native anonymous (only Gecko has native - /// anonymous content). - fn is_native_anonymous(&self) -> bool { + /// Returns true if this element is in a native anonymous subtree. + fn is_in_native_anonymous_subtree(&self) -> bool { false } @@ -794,7 +787,7 @@ pub trait TElement: /// element. fn rule_hash_target(&self) -> Self { if self.implemented_pseudo_element().is_some() { - self.closest_non_native_anonymous_ancestor() + self.pseudo_element_originating_element() .expect("Trying to collect rules for a detached pseudo-element") } else { *self diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 4b8148bb701..562d6458aa3 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -588,6 +588,20 @@ impl<'le> fmt::Debug for GeckoElement<'le> { } impl<'le> GeckoElement<'le> { + #[inline] + fn closest_anon_subtree_root_parent(&self) -> Option { + debug_assert!(self.is_in_native_anonymous_subtree()); + let mut current = *self; + + loop { + if current.is_root_of_native_anonymous_subtree() { + return current.traversal_parent(); + } + + current = current.traversal_parent()?; + } + } + #[inline] fn may_have_anonymous_children(&self) -> bool { self.as_node() @@ -813,13 +827,6 @@ impl<'le> GeckoElement<'le> { return self.flags() & (NODE_IS_NATIVE_ANONYMOUS_ROOT as u32) != 0; } - /// This logic is duplicated in Gecko's nsINode::IsInNativeAnonymousSubtree. - #[inline] - fn is_in_native_anonymous_subtree(&self) -> bool { - use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE; - self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) != 0 - } - /// This logic is duplicated in Gecko's nsIContent::IsInAnonymousSubtree. #[inline] fn is_in_anonymous_subtree(&self) -> bool { @@ -1038,13 +1045,11 @@ impl<'le> TElement for GeckoElement<'le> { type TraversalChildrenIterator = GeckoChildrenIterator<'le>; fn inheritance_parent(&self) -> Option { - if self.is_native_anonymous() { - self.closest_non_native_anonymous_ancestor() - } else { - self.as_node() - .flattened_tree_parent() - .and_then(|n| n.as_element()) + if self.implemented_pseudo_element().is_some() { + return self.pseudo_element_originating_element() } + + self.as_node().flattened_tree_parent().and_then(|n| n.as_element()) } fn traversal_children(&self) -> LayoutIterator> { @@ -1174,19 +1179,6 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { bindings::Gecko_DestroyAnonymousContentList(array) }; } - fn closest_non_native_anonymous_ancestor(&self) -> Option { - debug_assert!(self.is_native_anonymous()); - let mut parent = self.traversal_parent()?; - - loop { - if !parent.is_native_anonymous() { - return Some(parent); - } - - parent = parent.traversal_parent()?; - } - } - #[inline] fn as_node(&self) -> Self::ConcreteNode { unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) } @@ -1354,10 +1346,11 @@ impl<'le> TElement for GeckoElement<'le> { self.state().intersects(ElementState::IN_VISITED_STATE) } + /// This logic is duplicated in Gecko's nsINode::IsInNativeAnonymousSubtree. #[inline] - fn is_native_anonymous(&self) -> bool { - use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS; - self.flags() & (NODE_IS_NATIVE_ANONYMOUS as u32) != 0 + fn is_in_native_anonymous_subtree(&self) -> bool { + use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE; + self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) != 0 } #[inline] @@ -1366,7 +1359,7 @@ impl<'le> TElement for GeckoElement<'le> { } fn implemented_pseudo_element(&self) -> Option { - if !self.is_native_anonymous() { + if !self.is_in_native_anonymous_subtree() { return None; } @@ -1915,7 +1908,22 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn pseudo_element_originating_element(&self) -> Option { debug_assert!(self.implemented_pseudo_element().is_some()); - self.closest_non_native_anonymous_ancestor() + let parent = self.closest_anon_subtree_root_parent()?; + + // FIXME(emilio): Special-case for s + // pseudo-elements, which are nested NAC. Probably nsNumberControlFrame + // should instead inherit from nsTextControlFrame, and then this could + // go away. + if let Some(PseudoElement::MozNumberText) = parent.implemented_pseudo_element() { + debug_assert_eq!( + self.implemented_pseudo_element().unwrap(), + PseudoElement::Placeholder, + "You added a new pseudo, do you really want this?" + ); + return parent.closest_anon_subtree_root_parent(); + } + + Some(parent) } #[inline] diff --git a/components/style/matching.rs b/components/style/matching.rs index 1d5cbf8d375..05630b07fb7 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -678,7 +678,7 @@ pub trait MatchMethods: TElement { let new_primary_style = data.styles.primary.as_ref().unwrap(); let mut cascade_requirement = ChildCascadeRequirement::CanSkipCascade; - if self.is_root() && !self.is_native_anonymous() { + if self.is_root() && !self.is_in_native_anonymous_subtree() { let device = context.shared.stylist.device(); let new_font_size = new_primary_style.get_font().clone_font_size(); diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 4a9c6d5408c..314e9877083 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -565,7 +565,7 @@ impl StyleSharingCache { }, }; - if element.is_native_anonymous() { + if element.is_in_native_anonymous_subtree() { debug!("Failing to insert into the cache: NAC"); return; } @@ -656,7 +656,7 @@ impl StyleSharingCache { return None; } - if target.is_native_anonymous() { + if target.is_in_native_anonymous_subtree() { debug!("{:?} Cannot share style: NAC", target.element); return None; } @@ -681,7 +681,7 @@ impl StyleSharingCache { nth_index_cache: &mut NthIndexCache, selector_flags_map: &mut SelectorFlagsMap, ) -> Option { - debug_assert!(!target.is_native_anonymous()); + debug_assert!(!target.is_in_native_anonymous_subtree()); // Check that we have the same parent, or at least that the parents // share styles and permit sharing across their children. The latter diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index ab71cdb9bc0..b31beff7460 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -191,7 +191,9 @@ where // Before doing the cascade, check the sharing cache and see if we can // reuse the style via rule node identity. let may_reuse = - !self.element.is_native_anonymous() && parent_style.is_some() && inputs.rules.is_some(); + !self.element.is_in_native_anonymous_subtree() && + parent_style.is_some() && + inputs.rules.is_some(); if may_reuse { let cached = self.context.thread_local.sharing_cache.lookup_by_rules( diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 7092101fff7..0bffd39819c 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -433,9 +433,10 @@ pub fn recalc_style_at( if compute_self { child_cascade_requirement = compute_style(traversal_data, context, element, data); - if element.is_native_anonymous() { - // We must always cascade native anonymous subtrees, since they inherit - // styles from their first non-NAC ancestor. + if element.is_in_native_anonymous_subtree() { + // We must always cascade native anonymous subtrees, since they + // may have pseudo-elements underneath that would inherit from the + // closest non-NAC ancestor instead of us. child_cascade_requirement = cmp::max( child_cascade_requirement, ChildCascadeRequirement::MustCascadeChildren,