From 5de2f29080ca7a8d40c2c1c1a53fb6ec041c95ce Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 31 May 2017 18:03:33 -0700 Subject: [PATCH] stylo: Create separate Attr type --- components/style/properties/gecko.mako.rs | 8 +- .../properties/longhand/counters.mako.rs | 58 ++-------- components/style/stylesheets.rs | 20 ++-- components/style/values/specified/mod.rs | 109 +++++++++++++++++- tests/unit/style/parsing/selectors.rs | 2 +- tests/unit/style/stylesheets.rs | 2 +- 6 files changed, 132 insertions(+), 67 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 4871d1d5a72..7d9db1fd399 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4401,12 +4401,12 @@ clip-path as_utf16_and_forget(&value); } } - ContentItem::Attr(ns, val) => { + ContentItem::Attr(attr) => { self.gecko.mContents[i].mType = eStyleContentType_Attr; - let s = if let Some((_, ns)) = ns { - format!("{}|{}", ns, val) + let s = if let Some((_, ns)) = attr.namespace { + format!("{}|{}", ns, attr.attribute) } else { - val.into() + attr.attribute.into() }; unsafe { // NB: we share allocators, so doing this is fine. diff --git a/components/style/properties/longhand/counters.mako.rs b/components/style/properties/longhand/counters.mako.rs index fec8ff10822..e71d80c42af 100644 --- a/components/style/properties/longhand/counters.mako.rs +++ b/components/style/properties/longhand/counters.mako.rs @@ -15,7 +15,7 @@ #[cfg(feature = "gecko")] use values::specified::url::SpecifiedUrl; #[cfg(feature = "gecko")] - use gecko_string_cache::namespace::Namespace; + use values::specified::Attr; #[cfg(feature = "servo")] use super::list_style_type; @@ -37,8 +37,9 @@ type CounterStyleType = super::super::list_style_type::computed_value::T; #[cfg(feature = "gecko")] type CounterStyleType = ::values::generics::CounterStyleOrNone; + #[cfg(feature = "gecko")] - use gecko_string_cache::namespace::Namespace; + use values::specified::Attr; #[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -60,7 +61,7 @@ % if product == "gecko": /// `attr([namespace? `|`]? ident)` - Attr(Option<(Namespace, u32)>, String), + Attr(Attr), /// `url(url)` Url(SpecifiedUrl), % endif @@ -94,14 +95,8 @@ ContentItem::NoCloseQuote => dest.write_str("no-close-quote"), % if product == "gecko": - ContentItem::Attr(ref ns, ref attr) => { - dest.write_str("attr(")?; - if let Some(ref ns) = *ns { - cssparser::Token::Ident(ns.0.to_string().into()).to_css(dest)?; - dest.write_str("|")?; - } - cssparser::Token::Ident((&**attr).into()).to_css(dest)?; - dest.write_str(")") + ContentItem::Attr(ref attr) => { + attr.to_css(dest) } ContentItem::Url(ref url) => url.to_css(dest), % endif @@ -207,46 +202,7 @@ }), % if product == "gecko": "attr" => input.parse_nested_block(|input| { - // Syntax is `[namespace? `|`]? ident` - // no spaces allowed - // FIXME (bug 1346693) we should be checking that - // this is a valid namespace and encoding it as a namespace - // number from the map - let first = input.try(|i| i.expect_ident()).ok(); - if let Ok(token) = input.try(|i| i.next_including_whitespace()) { - match token { - Token::Delim('|') => { - // must be followed by an ident - let tok2 = input.next_including_whitespace()?; - if let Token::Ident(second) = tok2 { - let first: Option = first.map(|i| i.into()); - let first_with_id = match (first, context.namespaces) { - (Some(prefix), Some(map)) => { - let map = map.read(); - if let Some(ref entry) = map.prefixes.get(&prefix.0) { - Some((prefix, entry.1)) - } else { - return Err(()) - } - } - // if we don't have a namespace map (e.g. in CSSOM) - // we can't parse namespaces - (Some(_), None) => return Err(()), - _ => None - }; - return Ok(ContentItem::Attr(first_with_id, second.into_owned())) - } else { - return Err(()) - } - } - _ => return Err(()) - } - } - if let Some(first) = first { - Ok(ContentItem::Attr(None, first.into_owned())) - } else { - Err(()) - } + Ok(ContentItem::Attr(Attr::parse_function(context, input)?)) }), % endif _ => return Err(()) diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index de49dee5754..3a0da395ec8 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -51,6 +51,7 @@ use stylearc::Arc; use stylist::FnvHashMap; use supports::SupportsCondition; use values::{CustomIdent, KeyframesName}; +use values::specified::NamespaceId; use values::specified::url::SpecifiedUrl; use viewport::ViewportRule; @@ -97,12 +98,12 @@ pub enum Origin { /// A set of namespaces applying to a given stylesheet. /// -/// The u32 is the namespace id, used in gecko +/// The namespace id is used in gecko #[derive(Clone, Default, Debug)] #[allow(missing_docs)] pub struct Namespaces { - pub default: Option<(Namespace, u32)>, - pub prefixes: FnvHashMap, + pub default: Option<(Namespace, NamespaceId)>, + pub prefixes: FnvHashMap, } /// Like gecko_bindings::structs::MallocSizeOf, but without the Option<> wrapper. Note that @@ -1240,8 +1241,9 @@ impl Stylesheet { let mut rules = Vec::new(); let mut input = Parser::new(css); let mut context = ParserContext::new_with_line_number_offset(origin, url_data, error_reporter, - line_number_offset, PARSING_MODE_DEFAULT, - quirks_mode); + line_number_offset, + PARSING_MODE_DEFAULT, + quirks_mode); context.namespaces = Some(namespaces); let rule_parser = TopLevelRuleParser { stylesheet_origin: origin, @@ -1529,18 +1531,18 @@ enum AtRulePrelude { #[cfg(feature = "gecko")] -fn register_namespace(ns: &Namespace) -> Result { +fn register_namespace(ns: &Namespace) -> Result { let id = unsafe { ::gecko_bindings::bindings::Gecko_RegisterNamespace(ns.0.as_ptr()) }; if id == -1 { Err(()) } else { - Ok(id as u32) + Ok(id) } } #[cfg(feature = "servo")] -fn register_namespace(ns: &Namespace) -> Result { - Ok(1) // servo doesn't use namespace ids +fn register_namespace(_: &Namespace) -> Result<(), ()> { + Ok(()) // servo doesn't use namespace ids } impl<'a> AtRuleParser for TopLevelRuleParser<'a> { diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 3913c5914e8..7eb637315d3 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -6,9 +6,10 @@ //! //! TODO(emilio): Enhance docs. +use Namespace; use app_units::Au; use context::QuirksMode; -use cssparser::{self, Parser, Token}; +use cssparser::{self, Parser, Token, serialize_identifier}; use itoa; use parser::{ParserContext, Parse}; use self::grid::TrackSizeOrRepeat; @@ -24,6 +25,7 @@ use super::computed::{self, Context}; use super::computed::{Shadow as ComputedShadow, ToComputedValue}; use super::generics::grid::{TrackBreadth as GenericTrackBreadth, TrackSize as GenericTrackSize}; use super::generics::grid::TrackList as GenericTrackList; +use values::computed::ComputedValueAsSpecified; use values::specified::calc::CalcNode; #[cfg(feature = "gecko")] @@ -1363,3 +1365,108 @@ impl AllowQuirks { self == AllowQuirks::Yes && quirks_mode == QuirksMode::Quirks } } + +#[cfg(feature = "gecko")] +/// A namespace ID +pub type NamespaceId = i32; + + +#[cfg(feature = "servo")] +/// A namespace ID (used by gecko only) +pub type NamespaceId = (); + +/// An attr(...) rule +/// +/// `[namespace? `|`]? ident` +#[derive(Clone, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub struct Attr { + /// Optional namespace + pub namespace: Option<(Namespace, NamespaceId)>, + /// Attribute name + pub attribute: String, +} + +impl Parse for Attr { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + input.expect_function_matching("attr")?; + input.parse_nested_block(|i| Attr::parse_function(context, i)) + } +} + +#[cfg(feature = "gecko")] +/// Get the namespace id from the namespace map +pub fn get_id_for_namespace(namespace: &Namespace, context: &ParserContext) -> Result { + if let Some(map) = context.namespaces { + if let Some(ref entry) = map.read().prefixes.get(&namespace.0) { + Ok(entry.1) + } else { + Err(()) + } + } else { + // if we don't have a namespace map (e.g. in inline styles) + // we can't parse namespaces + Err(()) + } +} + +#[cfg(feature = "servo")] +/// Get the namespace id from the namespace map +pub fn get_id_for_namespace(_: &Namespace, _: &ParserContext) -> Result { + Ok(()) +} + +impl Attr { + /// Parse contents of attr() assuming we have already parsed `attr` and are + /// within a parse_nested_block() + pub fn parse_function(context: &ParserContext, input: &mut Parser) -> Result { + // Syntax is `[namespace? `|`]? ident` + // no spaces allowed + let first = input.try(|i| i.expect_ident()).ok(); + if let Ok(token) = input.try(|i| i.next_including_whitespace()) { + match token { + Token::Delim('|') => { + // must be followed by an ident + let second_token = match input.next_including_whitespace()? { + Token::Ident(second) => second, + _ => return Err(()), + }; + let ns_with_id = if let Some(ns) = first { + let ns: Namespace = ns.into(); + let id = get_id_for_namespace(&ns, context)?; + Some((ns, id)) + } else { + None + }; + return Ok(Attr { + namespace: ns_with_id, + attribute: second_token.into_owned(), + }) + } + _ => return Err(()) + } + } + if let Some(first) = first { + Ok(Attr { + namespace: None, + attribute: first.into_owned(), + }) + } else { + Err(()) + } + } +} + +impl ToCss for Attr { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + dest.write_str("attr(")?; + if let Some(ref ns) = self.namespace { + serialize_identifier(&ns.0.to_string(), dest)?; + dest.write_str("|")?; + } + serialize_identifier(&self.attribute, dest)?; + dest.write_str(")") + } +} + +impl ComputedValueAsSpecified for Attr {} diff --git a/tests/unit/style/parsing/selectors.rs b/tests/unit/style/parsing/selectors.rs index 08c8217d8fa..b865b39e870 100644 --- a/tests/unit/style/parsing/selectors.rs +++ b/tests/unit/style/parsing/selectors.rs @@ -9,7 +9,7 @@ use style::stylesheets::{Origin, Namespaces}; fn parse_selector(input: &mut Parser) -> Result, ()> { let mut ns = Namespaces::default(); - ns.prefixes.insert("svg".into(), ns!(svg)); + ns.prefixes.insert("svg".into(), (ns!(svg), ())); let parser = SelectorParser { stylesheet_origin: Origin::UserAgent, namespaces: &ns, diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 7ca36677429..a161c41cabe 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -70,7 +70,7 @@ fn test_parse_stylesheet() { let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock, None, &CSSErrorReporterTest, QuirksMode::NoQuirks, 0u64); let mut namespaces = Namespaces::default(); - namespaces.default = Some(ns!(html)); + namespaces.default = Some((ns!(html), ())); let expected = Stylesheet { origin: Origin::UserAgent, media: Arc::new(stylesheet.shared_lock.wrap(MediaList::empty())),