From 4f525f6fa04195cdb93fc4394fbc0c78f2626bd7 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sun, 30 Jul 2017 13:32:40 +0800 Subject: [PATCH 1/2] style: Preserve font-family identifier sequence when serializing. --- components/style/properties/gecko.mako.rs | 20 ++++---- .../style/properties/longhand/font.mako.rs | 46 ++++++++++++++----- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 24bf2471633..22e725a1679 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1782,7 +1782,7 @@ fn static_assert() { } pub fn set_font_family(&mut self, v: longhands::font_family::computed_value::T) { - use properties::longhands::font_family::computed_value::FontFamily; + use properties::longhands::font_family::computed_value::{FontFamily, FamilyNameSyntax}; let list = &mut self.gecko.mFont.fontlist; unsafe { Gecko_FontFamilyList_Clear(list); } @@ -1792,7 +1792,8 @@ fn static_assert() { for family in &v.0 { match *family { FontFamily::FamilyName(ref f) => { - unsafe { Gecko_FontFamilyList_AppendNamed(list, f.name.as_ptr(), f.quoted); } + let quoted = matches!(f.syntax, FamilyNameSyntax::Quoted); + unsafe { Gecko_FontFamilyList_AppendNamed(list, f.name.as_ptr(), quoted); } } FontFamily::Generic(ref name) => { let (family_type, generic) = FontFamily::generic(name); @@ -1819,7 +1820,7 @@ fn static_assert() { } pub fn clone_font_family(&self) -> longhands::font_family::computed_value::T { - use properties::longhands::font_family::computed_value::{FontFamily, FamilyName}; + use properties::longhands::font_family::computed_value::{FontFamily, FamilyName, FamilyNameSyntax}; use gecko_bindings::structs::FontFamilyType; use gecko_string_cache::Atom; @@ -1832,13 +1833,16 @@ fn static_assert() { FontFamilyType::eFamily_cursive => FontFamily::Generic(atom!("cursive")), FontFamilyType::eFamily_fantasy => FontFamily::Generic(atom!("fantasy")), FontFamilyType::eFamily_moz_fixed => FontFamily::Generic(Atom::from("-moz-fixed")), - FontFamilyType::eFamily_named => FontFamily::FamilyName(FamilyName { - name: (&*gecko_font_family_name.mName).into(), - quoted: false - }), + FontFamilyType::eFamily_named => { + let name = Atom::from(&*gecko_font_family_name.mName); + FontFamily::FamilyName(FamilyName { + name: name.clone(), + syntax: FamilyNameSyntax::Identifiers(vec![name]), + }) + }, FontFamilyType::eFamily_named_quoted => FontFamily::FamilyName(FamilyName { name: (&*gecko_font_family_name.mName).into(), - quoted: true + syntax: FamilyNameSyntax::Quoted, }), x => panic!("Found unexpected font FontFamilyType: {:?}", x), } diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index f6b78a91749..e47600a8a4e 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -96,7 +96,14 @@ macro_rules! impl_gecko_keyword_conversions { #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))] pub struct FamilyName { pub name: Atom, - pub quoted: bool, + pub syntax: FamilyNameSyntax, + } + + #[derive(Debug, PartialEq, Eq, Clone, Hash)] + #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))] + pub enum FamilyNameSyntax { + Quoted, + Identifiers(Vec), } impl FontFamily { @@ -139,7 +146,7 @@ macro_rules! impl_gecko_keyword_conversions { // quoted by default. FontFamily::FamilyName(FamilyName { name: input, - quoted: true, + syntax: FamilyNameSyntax::Quoted, }) } @@ -148,10 +155,14 @@ macro_rules! impl_gecko_keyword_conversions { if let Ok(value) = input.try(|i| i.expect_string_cloned()) { return Ok(FontFamily::FamilyName(FamilyName { name: Atom::from(&*value), - quoted: true, + syntax: FamilyNameSyntax::Quoted, })) } + + let mut identifiers = vec![]; + let first_ident = input.expect_ident()?.clone(); + identifiers.push(Atom::from(&*first_ident)); // FIXME(bholley): The fast thing to do here would be to look up the // string (as lowercase) in the static atoms table. We don't have an @@ -188,14 +199,16 @@ macro_rules! impl_gecko_keyword_conversions { let ident = input.expect_ident()?; value.push_str(" "); value.push_str(&ident); + identifiers.push(Atom::from(&*ident.clone())); } while let Ok(ident) = input.try(|i| i.expect_ident_cloned()) { value.push_str(" "); value.push_str(&ident); + identifiers.push(Atom::from(&*ident)); } Ok(FontFamily::FamilyName(FamilyName { name: Atom::from(value), - quoted: false, + syntax: FamilyNameSyntax::Identifiers(identifiers), })) } @@ -229,12 +242,23 @@ macro_rules! impl_gecko_keyword_conversions { impl ToCss for FamilyName { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if self.quoted { - dest.write_char('"')?; - write!(CssStringWriter::new(dest), "{}", self.name)?; - dest.write_char('"') - } else { - serialize_identifier(&*self.name.to_string(), dest) + match self.syntax { + FamilyNameSyntax::Quoted => { + dest.write_char('"')?; + write!(CssStringWriter::new(dest), "{}", self.name)?; + dest.write_char('"') + } + FamilyNameSyntax::Identifiers(ref identifiers) => { + let mut first = true; + for identifier in identifiers { + if !first { + dest.write_char(' ')?; + } + serialize_identifier(&*identifier.to_string(), dest)?; + first = false; + } + Ok(()) + } } } } @@ -2471,7 +2495,7 @@ ${helpers.single_keyword("-moz-math-variant", use properties::longhands::font_family::computed_value::*; FontFamily::FamilyName(FamilyName { name: (&*font.mName).into(), - quoted: true + syntax: FamilyNameSyntax::Quoted, }) }).collect::>(); let weight = longhands::font_weight::computed_value::T::from_gecko_weight(system.weight); From bb44c0a6bcc5e2c4be26d8187769de875b4326da Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 4 Aug 2017 19:33:07 +1000 Subject: [PATCH 2/2] style: Allow styles to be computed ignoring existing element data. --- components/layout/query.rs | 2 +- components/style/gecko/generated/bindings.rs | 3 ++- components/style/traversal.rs | 4 +++- ports/geckolib/glue.rs | 10 ++++++---- tests/unit/gfx/font_cache_thread.rs | 6 +++--- tests/unit/style/stylesheets.rs | 4 ++-- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index 252071e9bcd..fd9a97f44cc 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -702,7 +702,7 @@ pub fn process_resolved_style_request<'a, N>(context: &LayoutContext, thread_local: &mut tlc, }; - let styles = resolve_style(&mut context, element, RuleInclusion::All); + let styles = resolve_style(&mut context, element, RuleInclusion::All, false); let style = styles.primary(); let longhand_id = match *property { PropertyId::Longhand(id) => id, diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 5c275e666f0..47ad8441b8c 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -2810,7 +2810,8 @@ extern "C" { rule_inclusion: StyleRuleInclusion, snapshots: *const ServoElementSnapshotTable, - set: RawServoStyleSetBorrowed) + set: RawServoStyleSetBorrowed, + ignore_existing_styles: bool) -> ServoStyleContextStrong; } extern "C" { diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 7fc5cf22609..5197752cdd5 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -388,6 +388,7 @@ pub fn resolve_style( context: &mut StyleContext, element: E, rule_inclusion: RuleInclusion, + ignore_existing_style: bool, ) -> ElementStyles where E: TElement, @@ -395,6 +396,7 @@ where use style_resolver::StyleResolverForElement; debug_assert!(rule_inclusion == RuleInclusion::DefaultOnly || + ignore_existing_style || element.borrow_data().map_or(true, |d| !d.has_styles()), "Why are we here?"); let mut ancestors_requiring_style_resolution = SmallVec::<[E; 16]>::new(); @@ -405,7 +407,7 @@ where let mut style = None; let mut ancestor = element.traversal_parent(); while let Some(current) = ancestor { - if rule_inclusion == RuleInclusion::All { + if rule_inclusion == RuleInclusion::All && !ignore_existing_style { if let Some(data) = current.borrow_data() { if let Some(ancestor_style) = data.styles.get_primary() { style = Some(ancestor_style.clone()); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index ea4297b97c6..8b3320b158f 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2857,7 +2857,8 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed, pseudo_type: CSSPseudoElementType, rule_inclusion: StyleRuleInclusion, snapshots: *const ServoElementSnapshotTable, - raw_data: RawServoStyleSetBorrowed) + raw_data: RawServoStyleSetBorrowed, + ignore_existing_styles: bool) -> ServoStyleContextStrong { debug_assert!(!snapshots.is_null()); @@ -2888,8 +2889,9 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed, // In the common case we already have the style. Check that before setting // up all the computation machinery. (Don't use it when we're getting - // default styles, though.) - if rule_inclusion == RuleInclusion::All { + // default styles or in a bfcached document (as indicated by + // ignore_existing_styles), though.) + if rule_inclusion == RuleInclusion::All && !ignore_existing_styles { let styles = element.mutate_data().and_then(|d| { if d.has_styles() { Some(finish(&d.styles)) @@ -2914,7 +2916,7 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed, thread_local: &mut tlc, }; - let styles = resolve_style(&mut context, element, rule_inclusion); + let styles = resolve_style(&mut context, element, rule_inclusion, ignore_existing_styles); finish(&styles).into() } diff --git a/tests/unit/gfx/font_cache_thread.rs b/tests/unit/gfx/font_cache_thread.rs index 91e5b712dc7..e60ca8b9ba4 100644 --- a/tests/unit/gfx/font_cache_thread.rs +++ b/tests/unit/gfx/font_cache_thread.rs @@ -5,7 +5,7 @@ use cssparser::SourceLocation; use gfx::font_cache_thread::FontCacheThread; use ipc_channel::ipc; -use style::computed_values::font_family::FamilyName; +use style::computed_values::font_family::{FamilyName, FamilyNameSyntax}; use style::font_face::{FontFaceRuleData, Source}; #[test] @@ -15,11 +15,11 @@ fn test_local_web_font() { let font_cache_thread = FontCacheThread::new(inp_chan, None); let family_name = FamilyName { name: From::from("test family"), - quoted: true, + syntax: FamilyNameSyntax::Quoted, }; let variant_name = FamilyName { name: From::from("test font face"), - quoted: true, + syntax: FamilyNameSyntax::Quoted, }; let font_face_rule = FontFaceRuleData { family: Some(family_name.clone()), diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 4eb371317f9..a58567a02ea 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -14,7 +14,7 @@ use servo_url::ServoUrl; use std::borrow::ToOwned; use std::sync::Mutex; use std::sync::atomic::AtomicBool; -use style::computed_values::font_family::FamilyName; +use style::computed_values::font_family::{FamilyName, FamilyNameSyntax}; use style::context::QuirksMode; use style::error_reporting::{ParseErrorReporter, ContextualParseError}; use style::media_queries::MediaList; @@ -254,7 +254,7 @@ fn test_parse_stylesheet() { CssRule::FontFeatureValues(Arc::new(stylesheet.shared_lock.wrap(FontFeatureValuesRule { family_names: vec![FamilyName { name: Atom::from("test"), - quoted: false, + syntax: FamilyNameSyntax::Identifiers(vec![Atom::from("test")]), }], swash: vec![ FFVDeclaration {