From afe484e46b631689357de183020419ab1a49dcdf Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 20 Feb 2018 15:52:13 -0800 Subject: [PATCH 1/7] style: Allow placeholder import sheets. This is necessary because we can't create GeckoStyleSheets off-main-thread, so we need a placeholder until it can be filled in. Bug: 1454030 Reviewed-by: emilio MozReview-Commit-ID: ssRme4fLYg --- components/style/gecko/data.rs | 31 +++++-- components/style/stylesheet_set.rs | 2 +- components/style/stylesheets/import_rule.rs | 91 ++++++++++++++++--- .../style/stylesheets/rules_iterator.rs | 12 +-- components/style/stylesheets/stylesheet.rs | 80 ++++++++-------- ports/geckolib/glue.rs | 2 +- ports/geckolib/stylesheet_loader.rs | 5 +- 7 files changed, 152 insertions(+), 71 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 9c58a292dfc..331b07ea551 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -5,6 +5,7 @@ //! Data needed to style a Gecko document. use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; +use context::QuirksMode; use dom::TElement; use gecko_bindings::bindings::{self, RawServoStyleSet}; use gecko_bindings::structs::{self, RawGeckoPresContextOwned, ServoStyleSetSizes, ServoStyleSheet}; @@ -17,7 +18,7 @@ use properties::ComputedValues; use selector_parser::SnapshotMap; use servo_arc::Arc; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; -use stylesheets::{StylesheetContents, StylesheetInDocument}; +use stylesheets::{CssRule, Origin, StylesheetContents, StylesheetInDocument}; use stylist::Stylist; /// Little wrapper to a Gecko style sheet. @@ -58,6 +59,16 @@ impl GeckoStyleSheet { &*(self.raw()._base.mInner as *const StyleSheetInfo as *const ServoStyleSheetInner) } } + + /// Gets the StylesheetContents for this stylesheet. + pub fn contents(&self) -> &StylesheetContents { + debug_assert!(!self.inner().mContents.mRawPtr.is_null()); + unsafe { + let contents = + (&**StylesheetContents::as_arc(&&*self.inner().mContents.mRawPtr)) as *const _; + &*contents + } + } } impl Drop for GeckoStyleSheet { @@ -74,13 +85,12 @@ impl Clone for GeckoStyleSheet { } impl StylesheetInDocument for GeckoStyleSheet { - fn contents(&self, _: &SharedRwLockReadGuard) -> &StylesheetContents { - debug_assert!(!self.inner().mContents.mRawPtr.is_null()); - unsafe { - let contents = - (&**StylesheetContents::as_arc(&&*self.inner().mContents.mRawPtr)) as *const _; - &*contents - } + fn origin(&self, _guard: &SharedRwLockReadGuard) -> Origin { + self.contents().origin + } + + fn quirks_mode(&self, _guard: &SharedRwLockReadGuard) -> QuirksMode { + self.contents().quirks_mode } fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { @@ -103,6 +113,11 @@ impl StylesheetInDocument for GeckoStyleSheet { fn enabled(&self) -> bool { true } + + #[inline] + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + self.contents().rules(guard) + } } /// The container for data that a Servo-backed Gecko document needs to style diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 1091b6674a1..a3b6e5cdc17 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -478,7 +478,7 @@ where sheet: &S, guard: &SharedRwLockReadGuard, ) -> &mut SheetCollection { - let origin = sheet.contents(guard).origin; + let origin = sheet.origin(guard); self.collections.borrow_mut_for_origin(&origin) } diff --git a/components/style/stylesheets/import_rule.rs b/components/style/stylesheets/import_rule.rs index 75557ed50ee..5fe5f9549d2 100644 --- a/components/style/stylesheets/import_rule.rs +++ b/components/style/stylesheets/import_rule.rs @@ -6,6 +6,7 @@ //! //! [import]: https://drafts.csswg.org/css-cascade-3/#at-import +use context::QuirksMode; use cssparser::SourceLocation; use media_queries::MediaList; use shared_lock::{DeepCloneParams, DeepCloneWithLock}; @@ -13,13 +14,52 @@ use shared_lock::{SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard}; use std::fmt::{self, Write}; use str::CssStringWriter; use style_traits::{CssWriter, ToCss}; -use stylesheets::{StylesheetContents, StylesheetInDocument}; +use stylesheets::{CssRule, Origin, StylesheetInDocument}; use values::CssUrl; +/// With asynchronous stylesheet parsing, we can't synchronously create a +/// GeckoStyleSheet. So we use this placeholder instead. +#[derive(Clone, Debug)] +pub struct PendingSheet { + origin: Origin, + quirks_mode: QuirksMode, +} + /// A sheet that is held from an import rule. #[cfg(feature = "gecko")] #[derive(Debug)] -pub struct ImportSheet(pub ::gecko::data::GeckoStyleSheet); +pub enum ImportSheet { + /// A bonafide stylesheet. + Sheet(::gecko::data::GeckoStyleSheet), + /// An @import created while parsing off-main-thread, whose Gecko sheet has + /// yet to be created and attached. + Pending(PendingSheet), +} + +#[cfg(feature = "gecko")] +impl ImportSheet { + /// Creates a new ImportSheet from a GeckoStyleSheet. + pub fn new(sheet: ::gecko::data::GeckoStyleSheet) -> Self { + ImportSheet::Sheet(sheet) + } + + /// Creates a pending ImportSheet for a load that has not started yet. + pub fn new_pending(origin: Origin, quirks_mode: QuirksMode) -> Self { + ImportSheet::Pending(PendingSheet { + origin, + quirks_mode, + }) + } + + /// Returns a reference to the GeckoStyleSheet in this ImportSheet, if it + /// exists. + pub fn as_sheet(&self) -> Option<&::gecko::data::GeckoStyleSheet> { + match *self { + ImportSheet::Sheet(ref s) => Some(s), + ImportSheet::Pending(_) => None, + } + } +} #[cfg(feature = "gecko")] impl DeepCloneWithLock for ImportSheet { @@ -31,10 +71,15 @@ impl DeepCloneWithLock for ImportSheet { ) -> Self { use gecko::data::GeckoStyleSheet; use gecko_bindings::bindings; - let clone = unsafe { - bindings::Gecko_StyleSheet_Clone(self.0.raw() as *const _, params.reference_sheet) - }; - ImportSheet(unsafe { GeckoStyleSheet::from_addrefed(clone) }) + match *self { + ImportSheet::Sheet(ref s) => { + let clone = unsafe { + bindings::Gecko_StyleSheet_Clone(s.raw() as *const _, params.reference_sheet) + }; + ImportSheet::Sheet(unsafe { GeckoStyleSheet::from_addrefed(clone) }) + }, + ImportSheet::Pending(ref p) => ImportSheet::Pending(p.clone()), + } } } @@ -44,17 +89,39 @@ impl DeepCloneWithLock for ImportSheet { pub struct ImportSheet(pub ::servo_arc::Arc<::stylesheets::Stylesheet>); impl StylesheetInDocument for ImportSheet { - /// Get the media associated with this stylesheet. - fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { - self.0.media(guard) + fn origin(&self, _guard: &SharedRwLockReadGuard) -> Origin { + match *self { + ImportSheet::Sheet(ref s) => s.contents().origin, + ImportSheet::Pending(ref p) => p.origin, + } } - fn contents(&self, guard: &SharedRwLockReadGuard) -> &StylesheetContents { - self.0.contents(guard) + fn quirks_mode(&self, _guard: &SharedRwLockReadGuard) -> QuirksMode { + match *self { + ImportSheet::Sheet(ref s) => s.contents().quirks_mode, + ImportSheet::Pending(ref p) => p.quirks_mode, + } } fn enabled(&self) -> bool { - self.0.enabled() + match *self { + ImportSheet::Sheet(ref s) => s.enabled(), + ImportSheet::Pending(_) => true, + } + } + + fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { + match *self { + ImportSheet::Sheet(ref s) => s.media(guard), + ImportSheet::Pending(_) => None, + } + } + + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + match *self { + ImportSheet::Sheet(ref s) => s.contents().rules(guard), + ImportSheet::Pending(_) => &[], + } } } diff --git a/components/style/stylesheets/rules_iterator.rs b/components/style/stylesheets/rules_iterator.rs index 3d63a218ff4..81bf890b0fd 100644 --- a/components/style/stylesheets/rules_iterator.rs +++ b/components/style/stylesheets/rules_iterator.rs @@ -9,8 +9,9 @@ use media_queries::Device; use shared_lock::SharedRwLockReadGuard; use smallvec::SmallVec; use std::slice; -use stylesheets::{CssRule, CssRules, DocumentRule, ImportRule, MediaRule, SupportsRule}; +use stylesheets::{CssRule, DocumentRule, ImportRule, MediaRule, SupportsRule}; use stylesheets::StylesheetInDocument; +use stylesheets::import_rule::ImportSheet; /// An iterator over a list of rules. pub struct RulesIterator<'a, 'b, C> @@ -35,10 +36,10 @@ where device: &'a Device, quirks_mode: QuirksMode, guard: &'a SharedRwLockReadGuard<'b>, - rules: &'a CssRules, + rules: &'a [CssRule], ) -> Self { let mut stack = SmallVec::new(); - stack.push(rules.0.iter()); + stack.push(rules.iter()); Self { device: device, quirks_mode: quirks_mode, @@ -102,10 +103,7 @@ where } import_rule .stylesheet - .contents(self.guard) - .rules - .read_with(self.guard) - .0 + .rules(self.guard) .iter() }, CssRule::Document(ref doc_rule) => { diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index 4bccbb504c9..08c2cb05f7a 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -103,22 +103,10 @@ impl StylesheetContents { } } - /// Return an iterator using the condition `C`. + /// Returns a reference to the list of rules. #[inline] - pub fn iter_rules<'a, 'b, C>( - &'a self, - device: &'a Device, - guard: &'a SharedRwLockReadGuard<'b>, - ) -> RulesIterator<'a, 'b, C> - where - C: NestedRuleIterationCondition, - { - RulesIterator::new( - device, - self.quirks_mode, - guard, - &self.rules.read_with(guard), - ) + pub fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + &self.rules.read_with(guard).0 } /// Measure heap usage. @@ -189,32 +177,20 @@ macro_rules! rule_filter { /// A trait to represent a given stylesheet in a document. pub trait StylesheetInDocument { - /// Get the contents of this stylesheet. - fn contents(&self, guard: &SharedRwLockReadGuard) -> &StylesheetContents; - /// Get the stylesheet origin. - fn origin(&self, guard: &SharedRwLockReadGuard) -> Origin { - self.contents(guard).origin - } + fn origin(&self, guard: &SharedRwLockReadGuard) -> Origin; /// Get the stylesheet quirks mode. - fn quirks_mode(&self, guard: &SharedRwLockReadGuard) -> QuirksMode { - self.contents(guard).quirks_mode - } + fn quirks_mode(&self, guard: &SharedRwLockReadGuard) -> QuirksMode; + + /// Get whether this stylesheet is enabled. + fn enabled(&self) -> bool; /// Get the media associated with this stylesheet. fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList>; - /// Returns whether the style-sheet applies for the current device. - fn is_effective_for_device(&self, device: &Device, guard: &SharedRwLockReadGuard) -> bool { - match self.media(guard) { - Some(medialist) => medialist.evaluate(device, self.quirks_mode(guard)), - None => true, - } - } - - /// Get whether this stylesheet is enabled. - fn enabled(&self) -> bool; + /// Returns a reference to the list of rules in this stylesheet. + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule]; /// Return an iterator using the condition `C`. #[inline] @@ -226,7 +202,15 @@ pub trait StylesheetInDocument { where C: NestedRuleIterationCondition, { - self.contents(guard).iter_rules(device, guard) + RulesIterator::new(device, self.quirks_mode(guard), guard, self.rules(guard)) + } + + /// Returns whether the style-sheet applies for the current device. + fn is_effective_for_device(&self, device: &Device, guard: &SharedRwLockReadGuard) -> bool { + match self.media(guard) { + Some(medialist) => medialist.evaluate(device, self.quirks_mode(guard)), + None => true, + } } /// Return an iterator over the effective rules within the style-sheet, as @@ -255,8 +239,12 @@ pub trait StylesheetInDocument { } impl StylesheetInDocument for Stylesheet { - fn contents(&self, _: &SharedRwLockReadGuard) -> &StylesheetContents { - &self.contents + fn origin(&self, _guard: &SharedRwLockReadGuard) -> Origin { + self.contents.origin + } + + fn quirks_mode(&self, _guard: &SharedRwLockReadGuard) -> QuirksMode { + self.contents.quirks_mode } fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { @@ -266,6 +254,11 @@ impl StylesheetInDocument for Stylesheet { fn enabled(&self) -> bool { !self.disabled() } + + #[inline] + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + self.contents.rules(guard) + } } /// A simple wrapper over an `Arc`, with pointer comparison, and @@ -289,8 +282,12 @@ impl ToMediaListKey for DocumentStyleSheet { } impl StylesheetInDocument for DocumentStyleSheet { - fn contents(&self, guard: &SharedRwLockReadGuard) -> &StylesheetContents { - self.0.contents(guard) + fn origin(&self, guard: &SharedRwLockReadGuard) -> Origin { + self.0.origin(guard) + } + + fn quirks_mode(&self, guard: &SharedRwLockReadGuard) -> QuirksMode { + self.0.quirks_mode(guard) } fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { @@ -300,6 +297,11 @@ impl StylesheetInDocument for DocumentStyleSheet { fn enabled(&self) -> bool { self.0.enabled() } + + #[inline] + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + self.0.rules(guard) + } } impl Stylesheet { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index ce957398391..3a476640de1 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2048,7 +2048,7 @@ pub extern "C" fn Servo_ImportRule_GetSheet( rule: RawServoImportRuleBorrowed, ) -> *const ServoStyleSheet { read_locked_arc(rule, |rule: &ImportRule| { - rule.stylesheet.0.raw() as *const ServoStyleSheet + rule.stylesheet.as_sheet().unwrap().raw() as *const ServoStyleSheet }) } diff --git a/ports/geckolib/stylesheet_loader.rs b/ports/geckolib/stylesheet_loader.rs index 0addb4f9ef8..1dfe30e8bdb 100644 --- a/ports/geckolib/stylesheet_loader.rs +++ b/ports/geckolib/stylesheet_loader.rs @@ -55,9 +55,8 @@ impl StyleStylesheetLoader for StylesheetLoader { debug_assert!(!child_sheet.is_null(), "Import rules should always have a strong sheet"); - let stylesheet = unsafe { - ImportSheet(GeckoStyleSheet::from_addrefed(child_sheet)) - }; + let sheet = unsafe { GeckoStyleSheet::from_addrefed(child_sheet) }; + let stylesheet = ImportSheet::new(sheet); Arc::new(lock.wrap(ImportRule { url, source_location, stylesheet })) } } From 1bc30a67320ce90f2e07988925a340b457f1a43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 14 Apr 2018 18:38:29 +0200 Subject: [PATCH 2/7] style: Move unboxing to style, and handle display: contents before other suppressions. This also adopts the resolution of [1] while at it, and switches XUL to not support display: contents until a use case appears. This makes our behavior consistent both with the spec and also in terms of handling dynamic changes to stuff that would otherwise get suppressed. Also makes us consistent with both Blink and WebKit in terms of computed style. We were the only ones respecting "behaves as display: none" without actually computing to display: none. Will file a spec issue to get that changed. It also makes us match Blink and WebKit in terms of respecting display: contents before other suppressions, see the reftest which I didn't write as a WPT (because there's no spec supporting neither that or the opposite of what we do), where a element respects display: contents even though if it had any other kind of display value we'd suppress the frame for it and all the descendants since it's an SVG element in a non-SVG subtree. Also, this removes the page-break bit from the display: contents loop, which I think is harmless. As long as the tests under style are based in namespace id / node name / traversal parent, this should not make style sharing go wrong in any way, since that's the first style sharing check we do at [2]. The general idea under this change is making all nodes with computed style of display: contents actually honor it. Otherwise there's no way of making the setup sound except re-introducing something similar to all the state tracking removed in bug 1303605. [1]: https://github.com/w3c/csswg-drafts/issues/2167 [2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700 Bug: 1453702 Reviewed-by: mats, xidorn MozReview-Commit-ID: JoCKnGYEleD --- components/style/dom.rs | 9 +++ components/style/gecko/wrapper.rs | 24 ++++-- components/style/style_adjuster.rs | 116 ++++++++++++++++++++++++++--- 3 files changed, 131 insertions(+), 18 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index acc1c364ca8..bee4d88329e 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -434,6 +434,15 @@ pub trait TElement: /// Return whether this element is an element in the HTML namespace. fn is_html_element(&self) -> bool; + /// Return whether this element is an element in the MathML namespace. + fn is_mathml_element(&self) -> bool; + + /// Return whether this element is an element in the SVG namespace. + fn is_svg_element(&self) -> bool; + + /// Return whether this element is an element in the XUL namespace. + fn is_xul_element(&self) -> bool { false } + /// Return the list of slotted nodes of this node. fn slotted_nodes(&self) -> &[Self::ConcreteNode] { &[] diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 2764c7e73b3..c00a41049c2 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -673,11 +673,6 @@ impl<'le> GeckoElement<'le> { self.as_node().node_info().mInner.mNamespaceID } - #[inline] - fn is_xul_element(&self) -> bool { - self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32) - } - #[inline] fn has_id(&self) -> bool { self.as_node() @@ -826,7 +821,7 @@ impl<'le> GeckoElement<'le> { match self.parent_element() { Some(e) => { e.local_name() == &*local_name!("use") && - e.namespace() == &*ns!("http://www.w3.org/2000/svg") + e.is_svg_element() }, None => false, } @@ -1058,7 +1053,22 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn is_html_element(&self) -> bool { - self.namespace_id() == (structs::root::kNameSpaceID_XHTML as i32) + self.namespace_id() == structs::kNameSpaceID_XHTML as i32 + } + + #[inline] + fn is_mathml_element(&self) -> bool { + self.namespace_id() == structs::kNameSpaceID_MathML as i32 + } + + #[inline] + fn is_svg_element(&self) -> bool { + self.namespace_id() == structs::kNameSpaceID_SVG as i32 + } + + #[inline] + fn is_xul_element(&self) -> bool { + self.namespace_id() == structs::root::kNameSpaceID_XUL as i32 } /// Return the list of slotted nodes of this node. diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 10e62b87cc2..d86639300e9 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -5,6 +5,7 @@ //! A struct to encapsulate all the style fixups and flags propagations //! a computed style needs in order for it to adhere to the CSS spec. +use Atom; use app_units::Au; use dom::TElement; use properties::{self, CascadeFlags, ComputedValues, StyleBuilder}; @@ -23,6 +24,84 @@ pub struct StyleAdjuster<'a, 'b: 'a> { style: &'a mut StyleBuilder<'b>, } +#[cfg(feature = "gecko")] +fn is_topmost_svg_svg_element(e: E) -> bool +where + E: TElement, +{ + debug_assert!(e.is_svg_element()); + if e.local_name() != &*atom!("svg") { + return false; + } + + let parent = match e.traversal_parent() { + Some(n) => n, + None => return true, + }; + + if !parent.is_svg_element() { + return true; + } + + parent.local_name() == &*atom!("foreignObject") +} + +// https://drafts.csswg.org/css-display/#unbox +#[cfg(feature = "gecko")] +fn is_effective_display_none_for_display_contents(element: E) -> bool +where + E: TElement, +{ + // FIXME(emilio): This should be an actual static. + lazy_static! { + static ref SPECIAL_HTML_ELEMENTS: [Atom; 16] = [ + atom!("br"), atom!("wbr"), atom!("meter"), atom!("progress"), + atom!("canvas"), atom!("embed"), atom!("object"), atom!("audio"), + atom!("iframe"), atom!("img"), atom!("video"), atom!("frame"), + atom!("frameset"), atom!("input"), atom!("textarea"), + atom!("select"), + ]; + } + + // https://drafts.csswg.org/css-display/#unbox-svg + // + // There's a note about "Unknown elements", but there's not a good way to + // know what that means, or to get that information from here, and no other + // UA implements this either. + lazy_static! { + static ref SPECIAL_SVG_ELEMENTS: [Atom; 6] = [ + atom!("svg"), atom!("a"), atom!("g"), atom!("use"), + atom!("tspan"), atom!("textPath"), + ]; + } + + // https://drafts.csswg.org/css-display/#unbox-html + if element.is_html_element() { + let local_name = element.local_name(); + return SPECIAL_HTML_ELEMENTS.iter().any(|name| &**name == local_name); + } + + // https://drafts.csswg.org/css-display/#unbox-svg + if element.is_svg_element() { + if is_topmost_svg_svg_element(element) { + return true; + } + let local_name = element.local_name(); + return !SPECIAL_SVG_ELEMENTS.iter().any(|name| &**name == local_name); + } + + // https://drafts.csswg.org/css-display/#unbox-mathml + // + // We always treat XUL as display: none. We don't use display: + // contents in XUL anyway, so should be fine to be consistent with + // MathML unless there's a use case for it. + if element.is_mathml_element() || element.is_xul_element() { + return true; + } + + false +} + impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// Trivially constructs a new StyleAdjuster. #[inline] @@ -377,20 +456,35 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } - /// Native anonymous content converts display:contents into display:inline. + /// Handles the relevant sections in: + /// + /// https://drafts.csswg.org/css-display/#unbox-html + /// + /// And forbidding display: contents in pseudo-elements, at least for now. #[cfg(feature = "gecko")] - fn adjust_for_prohibited_display_contents(&mut self) { - // TODO: We should probably convert display:contents into display:none - // in some cases too: https://drafts.csswg.org/css-display/#unbox - // - // FIXME(emilio): ::before and ::after should support display: contents, - // see bug 1418138. - if self.style.pseudo.is_none() || self.style.get_box().clone_display() != Display::Contents - { + fn adjust_for_prohibited_display_contents(&mut self, element: Option) + where + E: TElement, + { + if self.style.get_box().clone_display() != Display::Contents { return; } - self.style.mutate_box().set_display(Display::Inline); + // FIXME(emilio): ::before and ::after should support display: contents, + // see bug 1418138. + if self.style.pseudo.is_some() { + self.style.mutate_box().set_display(Display::Inline); + return; + } + + let element = match element { + Some(e) => e, + None => return, + }; + + if is_effective_display_none_for_display_contents(element) { + self.style.mutate_box().set_display(Display::None); + } } /// If a
has grid/flex display type, we need to inherit @@ -657,7 +751,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.adjust_for_visited(element); #[cfg(feature = "gecko")] { - self.adjust_for_prohibited_display_contents(); + self.adjust_for_prohibited_display_contents(element); self.adjust_for_fieldset_content(layout_parent_style); } self.adjust_for_top_layer(); From b11c3f1f45786dba53d9174d461a5f3f2ad1885e Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Tue, 17 Apr 2018 11:07:57 +1000 Subject: [PATCH 3/7] style: Remove trailing underscore of float ident. Bug: 1454528 Reviewed-by: heycam MozReview-Commit-ID: DN7rQu3adSB --- components/style/properties/data.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 823f611638b..4319551084e 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -300,10 +300,7 @@ class Longhand(object): return "<{} as ToAnimatedValue>::AnimatedValue".format(computed) def nscsspropertyid(self): - ident = self.ident - if ident == "float": - ident = "float_" - return "nsCSSPropertyID::eCSSProperty_%s" % ident + return "nsCSSPropertyID::eCSSProperty_%s" % self.ident class Shorthand(object): From 376aafc2c8638f0e8f399df5c8f980bef1b52425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Apr 2018 19:59:31 +0200 Subject: [PATCH 4/7] style: Update bindings. --- components/style/gecko/generated/structs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/gecko/generated/structs.rs b/components/style/gecko/generated/structs.rs index 0cf29c89cd6..ebe6b79afbc 100644 --- a/components/style/gecko/generated/structs.rs +++ b/components/style/gecko/generated/structs.rs @@ -18478,7 +18478,7 @@ pub mod root { eCSSProperty_flex_grow = 105, eCSSProperty_flex_shrink = 106, eCSSProperty_flex_wrap = 107, - eCSSProperty_float_ = 108, + eCSSProperty_float = 108, eCSSProperty__moz_float_edge = 109, eCSSProperty_flood_color = 110, eCSSProperty_flood_opacity = 111, From f5c85a816a44f31bdcf37f7a3b4dd4ed44627aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Apr 2018 20:00:36 +0200 Subject: [PATCH 5/7] style: Remove an unused import. --- components/style/stylesheets/rules_iterator.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/style/stylesheets/rules_iterator.rs b/components/style/stylesheets/rules_iterator.rs index 81bf890b0fd..b46e24c22c7 100644 --- a/components/style/stylesheets/rules_iterator.rs +++ b/components/style/stylesheets/rules_iterator.rs @@ -11,7 +11,6 @@ use smallvec::SmallVec; use std::slice; use stylesheets::{CssRule, DocumentRule, ImportRule, MediaRule, SupportsRule}; use stylesheets::StylesheetInDocument; -use stylesheets::import_rule::ImportSheet; /// An iterator over a list of rules. pub struct RulesIterator<'a, 'b, C> From 827b82dc398faa70b042d283ce9c65e7ea15da02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Apr 2018 19:55:25 +0200 Subject: [PATCH 6/7] style: Fix build bustage after afe484e46b631689357de183020419ab1a49dcdf. --- components/script/dom/document.rs | 18 ++++++++--- components/style/stylesheets/import_rule.rs | 34 ++++++++++++++++++--- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 01b9a13ba40..71df0e70356 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -135,7 +135,7 @@ use style::selector_parser::{RestyleDamage, Snapshot}; use style::shared_lock::{SharedRwLock as StyleSharedRwLock, SharedRwLockReadGuard}; use style::str::{split_html_space_chars, str_join}; use style::stylesheet_set::DocumentStylesheetSet; -use style::stylesheets::{Stylesheet, StylesheetContents, Origin, OriginSet}; +use style::stylesheets::{CssRule, Stylesheet, Origin, OriginSet}; use task_source::TaskSource; use time; use timers::OneshotTimerCallback; @@ -216,16 +216,24 @@ impl PartialEq for StyleSheetInDocument { } impl ::style::stylesheets::StylesheetInDocument for StyleSheetInDocument { - fn contents(&self, guard: &SharedRwLockReadGuard) -> &StylesheetContents { - self.sheet.contents(guard) + fn origin(&self, guard: &SharedRwLockReadGuard) -> Origin { + self.sheet.origin(guard) + } + + fn quirks_mode(&self, guard: &SharedRwLockReadGuard) -> QuirksMode { + self.sheet.quirks_mode(guard) + } + + fn enabled(&self) -> bool { + self.sheet.enabled() } fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { self.sheet.media(guard) } - fn enabled(&self) -> bool { - self.sheet.enabled() + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + self.sheet.rules(guard) } } diff --git a/components/style/stylesheets/import_rule.rs b/components/style/stylesheets/import_rule.rs index 5fe5f9549d2..fb50ec69732 100644 --- a/components/style/stylesheets/import_rule.rs +++ b/components/style/stylesheets/import_rule.rs @@ -83,11 +83,7 @@ impl DeepCloneWithLock for ImportSheet { } } -/// A sheet that is held from an import rule. -#[cfg(feature = "servo")] -#[derive(Debug)] -pub struct ImportSheet(pub ::servo_arc::Arc<::stylesheets::Stylesheet>); - +#[cfg(feature = "gecko")] impl StylesheetInDocument for ImportSheet { fn origin(&self, _guard: &SharedRwLockReadGuard) -> Origin { match *self { @@ -125,6 +121,34 @@ impl StylesheetInDocument for ImportSheet { } } +/// A sheet that is held from an import rule. +#[cfg(feature = "servo")] +#[derive(Debug)] +pub struct ImportSheet(pub ::servo_arc::Arc<::stylesheets::Stylesheet>); + +#[cfg(feature = "servo")] +impl StylesheetInDocument for ImportSheet { + fn origin(&self, guard: &SharedRwLockReadGuard) -> Origin { + self.0.origin(guard) + } + + fn quirks_mode(&self, guard: &SharedRwLockReadGuard) -> QuirksMode { + self.0.quirks_mode(guard) + } + + fn enabled(&self) -> bool { + self.0.enabled() + } + + fn media<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> Option<&'a MediaList> { + self.0.media(guard) + } + + fn rules<'a, 'b: 'a>(&'a self, guard: &'b SharedRwLockReadGuard) -> &'a [CssRule] { + self.0.rules(guard) + } +} + #[cfg(feature = "servo")] impl DeepCloneWithLock for ImportSheet { fn deep_clone_with_lock( From a82ed50257ebafe33dac5595606a4c4e548102b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Apr 2018 19:50:49 +0200 Subject: [PATCH 7/7] style: Fix build after 1bc30a67320ce90f2e07988925a340b457f1a43a. --- components/layout_thread/dom_wrapper.rs | 8 ++++++++ components/style/style_adjuster.rs | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 55bacd037cb..309ddaf7e93 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -397,6 +397,14 @@ impl<'le> TElement for ServoLayoutElement<'le> { unsafe { self.element.is_html_element() } } + fn is_mathml_element(&self) -> bool { + *self.element.namespace() == ns!(mathml) + } + + fn is_svg_element(&self) -> bool { + *self.element.namespace() == ns!(svg) + } + fn style_attribute(&self) -> Option>> { unsafe { (*self.element.style_attribute()).as_ref().map(|x| x.borrow_arc()) diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index d86639300e9..b7270976ead 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -5,7 +5,6 @@ //! A struct to encapsulate all the style fixups and flags propagations //! a computed style needs in order for it to adhere to the CSS spec. -use Atom; use app_units::Au; use dom::TElement; use properties::{self, CascadeFlags, ComputedValues, StyleBuilder}; @@ -52,6 +51,8 @@ fn is_effective_display_none_for_display_contents(element: E) -> bool where E: TElement, { + use Atom; + // FIXME(emilio): This should be an actual static. lazy_static! { static ref SPECIAL_HTML_ELEMENTS: [Atom; 16] = [