From bab6077c1c5729447cc4625886b7f72f257aaa32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 31 Dec 2017 16:59:17 +0100 Subject: [PATCH] Revert #19666 since we do create NAC elements and expect them to be inline. This reverts commit 1970e82b0d310128eabe8466d39d42cc20e7ae4b, reversing changes made to e882660ea694f9f12d9d2936012dbdf192f8aec8. The reparenting logic is still bogus, but I'll figure out how to deal with that in a bit. --- components/layout_thread/dom_wrapper.rs | 4 ++++ components/style/dom.rs | 5 +++++ components/style/gecko/wrapper.rs | 13 ++++++++++++ .../style/properties/properties.mako.rs | 14 ++++++++----- components/style/style_adjuster.rs | 21 +++++++------------ components/style/style_resolver.rs | 5 +++++ ports/geckolib/glue.rs | 16 +++++--------- 7 files changed, 49 insertions(+), 29 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 62b9ae8b7be..82a9678d7da 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -460,6 +460,10 @@ impl<'le> TElement for ServoLayoutElement<'le> { } } + fn skip_root_and_item_based_display_fixup(&self) -> bool { + false + } + unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) { self.element.insert_selector_flags(flags); } diff --git a/components/style/dom.rs b/components/style/dom.rs index 973a8172f77..90f4933b5ef 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -670,6 +670,11 @@ pub trait TElement self.get_data().map(|x| x.borrow_mut()) } + /// Whether we should skip any root- or item-based display property + /// blockification on this element. (This function exists so that Gecko + /// native anonymous content can opt out of this style fixup.) + fn skip_root_and_item_based_display_fixup(&self) -> bool; + /// Sets selector flags, which indicate what kinds of selectors may have /// matched on this element and therefore what kind of work may need to /// be performed when DOM state changes. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index a375cf08e12..5aa09092492 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1256,6 +1256,19 @@ impl<'le> TElement for GeckoElement<'le> { } } + #[inline] + fn skip_root_and_item_based_display_fixup(&self) -> bool { + if !self.is_native_anonymous() { + return false; + } + + if let Some(p) = self.implemented_pseudo_element() { + return p.skip_item_based_display_fixup(); + } + + self.is_root_of_native_anonymous_subtree() + } + unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) { debug_assert!(!flags.is_empty()); self.set_flags(selector_flags_to_node_flags(flags)); diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a837685777a..12750e33b2a 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -3060,10 +3060,14 @@ bitflags! { pub struct CascadeFlags: u8 { /// Whether to inherit all styles from the parent. If this flag is not /// present, non-inherited styles are reset to their initial values. - const INHERIT_ALL = 1 << 0; + const INHERIT_ALL = 1; + + /// Whether to skip any display style fixup for root element, flex/grid + /// item, and ruby descendants. + const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 1 << 1; /// Whether to only cascade properties that are visited dependent. - const VISITED_DEPENDENT_ONLY = 1 << 1; + const VISITED_DEPENDENT_ONLY = 1 << 2; /// Whether the given element we're styling is the document element, /// that is, matches :root. @@ -3073,15 +3077,15 @@ bitflags! { /// /// This affects some style adjustments, like blockification, and means /// that it may affect global state, like the Device's root font-size. - const IS_ROOT_ELEMENT = 1 << 2; + const IS_ROOT_ELEMENT = 1 << 3; /// Whether we're computing the style of a link, either visited or /// unvisited. - const IS_LINK = 1 << 3; + const IS_LINK = 1 << 4; /// Whether we're computing the style of a link element that happens to /// be visited. - const IS_VISITED_LINK = 1 << 4; + const IS_VISITED_LINK = 1 << 5; } } diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 74c4d309222..965d61af10a 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -28,12 +28,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { StyleAdjuster { style } } - /// Whether to skip any display style fixup flex/grid item, and ruby - /// descendants. - fn skip_item_based_display_fixup(&self) -> bool { - self.style.pseudo.as_ref().map_or(false, |p| p.skip_item_based_display_fixup()) - } - /// /// /// Any position value other than 'absolute' and 'fixed' are @@ -72,11 +66,10 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } - blockify_if!(flags.contains(CascadeFlags::IS_ROOT_ELEMENT)); - blockify_if!( - !self.skip_item_based_display_fixup() && - layout_parent_style.get_box().clone_display().is_item_container() - ); + if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) { + blockify_if!(flags.contains(CascadeFlags::IS_ROOT_ELEMENT)); + blockify_if!(layout_parent_style.get_box().clone_display().is_item_container()); + } let is_item_or_root = blockify; @@ -454,7 +447,9 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { fn adjust_for_ruby( &mut self, layout_parent_style: &ComputedValues, + flags: CascadeFlags, ) { + use properties::CascadeFlags; use properties::computed_value_flags::ComputedValueFlags; use properties::longhands::unicode_bidi::computed_value::T as UnicodeBidi; @@ -463,7 +458,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { if self.should_suppress_linebreak(layout_parent_style) { self.style.flags.insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); // Inlinify the display type if allowed. - if !self.skip_item_based_display_fixup() { + if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) { let inline_display = self_display.inlinify(); if self_display != inline_display { self.style.mutate_box().set_display(inline_display); @@ -599,7 +594,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.adjust_for_text_decoration_lines(layout_parent_style); #[cfg(feature = "gecko")] { - self.adjust_for_ruby(layout_parent_style); + self.adjust_for_ruby(layout_parent_style, flags); } self.set_bits(); } diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index 4caa3037c5b..65b816be854 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -542,6 +542,11 @@ where let mut cascade_flags = CascadeFlags::empty(); + if self.element.skip_root_and_item_based_display_fixup() || + pseudo.map_or(false, |p| p.skip_item_based_display_fixup()) { + cascade_flags.insert(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); + } + if pseudo.is_none() && self.element.is_link() { cascade_flags.insert(CascadeFlags::IS_LINK); if self.element.is_visited_link() && diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 7c7ac9f77a1..e12b4d1307d 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2019,11 +2019,12 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null: page_decls, ); + let cascade_flags = CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP; data.stylist.precomputed_values_for_pseudo_with_rule_node( &guards, &pseudo, parent_style_or_null.map(|x| &*x), - CascadeFlags::empty(), + cascade_flags, &metrics, rule_node ).into() @@ -3619,17 +3620,10 @@ pub extern "C" fn Servo_ReparentStyle( let element = element.map(GeckoElement); let mut cascade_flags = CascadeFlags::empty(); + if style_to_reparent.is_anon_box() { + cascade_flags.insert(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); + } if let Some(element) = element { - // NOTE(emilio): This relies on element.is_some() => pseudo.is_none(), - // which the caller guarantees, fortunately. But this doesn't handle the - // IS_ROOT_ELEMENT flag correctly! - // - // I think it's not possible to wrap a root element in a first-line - // frame (and reparenting only happens for ::first-line and its - // descendants), so this may be fine... - // - // We should just get rid of all these flags which pass element / pseudo - // state. if element.is_link() { cascade_flags.insert(CascadeFlags::IS_LINK); if element.is_visited_link() && doc_data.visited_styles_enabled() {