From 58d452fa4e7b4b426925fe3bc525251048ae2830 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 8 Dec 2016 16:17:04 -1000 Subject: [PATCH] Use PropertyId instead of Atom for CSSStyleDeclaration::get_computed_style --- components/atoms/static_atoms.txt | 15 ---- components/layout/query.rs | 86 ++++++++++--------- components/layout_thread/lib.rs | 4 +- components/script/dom/cssstyledeclaration.rs | 19 ++-- components/script/dom/window.rs | 13 +-- components/script_layout_interface/message.rs | 4 +- components/script_layout_interface/rpc.rs | 2 +- .../style/properties/properties.mako.rs | 25 ++++-- tests/unit/style/stylist.rs | 2 +- 9 files changed, 85 insertions(+), 85 deletions(-) diff --git a/components/atoms/static_atoms.txt b/components/atoms/static_atoms.txt index 2ac84aa2f4e..1ddf07db553 100644 --- a/components/atoms/static_atoms.txt +++ b/components/atoms/static_atoms.txt @@ -32,21 +32,6 @@ number dir -bottom -top -left -right -width -height -margin-bottom -margin-top -margin-left -margin-right -padding-bottom -padding-top -padding-left -padding-right - DOMContentLoaded select input diff --git a/components/layout/query.rs b/components/layout/query.rs index 765cd2bdab4..81d0ce5e320 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -24,7 +24,6 @@ use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElemen use script_traits::LayoutMsg as ConstellationMsg; use script_traits::UntrustedNodeAddress; use sequential; -use servo_atoms::Atom; use std::cmp::{min, max}; use std::ops::Deref; use std::sync::{Arc, Mutex}; @@ -32,8 +31,8 @@ use style::computed_values; use style::context::StyleContext; use style::dom::TElement; use style::logical_geometry::{WritingMode, BlockFlowDirection, InlineBaseDirection}; +use style::properties::{style_structs, PropertyId, PropertyDeclarationId, LonghandId}; use style::properties::longhands::{display, position}; -use style::properties::style_structs; use style::selector_parser::PseudoElement; use style::stylist::Stylist; use style_traits::ToCss; @@ -75,7 +74,7 @@ pub struct LayoutThreadData { pub scroll_area_response: Rect, /// A queued response for the resolved style property of an element. - pub resolved_style_response: Option, + pub resolved_style_response: String, /// A queued response for the offset parent/rect of a node. pub offset_parent_response: OffsetParentResponse, @@ -628,8 +627,8 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou pub fn process_resolved_style_request<'a, N, C>(requested_node: N, style_context: &'a C, pseudo: &Option, - property: &Atom, - layout_root: &mut Flow) -> Option + property: &PropertyId, + layout_root: &mut Flow) -> String where N: LayoutNode, C: StyleContext<'a> { @@ -667,13 +666,25 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, // The pseudo doesn't exist, return nothing. Chrome seems to query // the element itself in this case, Firefox uses the resolved value. // https://www.w3.org/Bugs/Public/show_bug.cgi?id=29006 - return None; + return String::new(); } Some(layout_el) => layout_el }; let style = &*layout_el.resolved_style(); + let longhand_id = match *property { + PropertyId::Longhand(id) => id, + + // Firefox returns blank strings for the computed value of shorthands, + // so this should be web-compatible. + PropertyId::Shorthand(_) => return String::new(), + + PropertyId::Custom(ref name) => { + return style.computed_value_to_string(PropertyDeclarationId::Custom(name)) + } + }; + // Clear any temporarily-resolved data to maintain our invariants. See the comment // at the top of this function. display_none_root.map(|r| clear_descendant_data(r, &|e| e.as_node().clear_data())); @@ -697,7 +708,7 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, layout_el: ::ConcreteThreadSafeLayoutElement, layout_root: &mut Flow, requested_node: N, - property: &Atom) -> Option { + longhand_id: LonghandId) -> String { let maybe_data = layout_el.borrow_layout_data(); let position = maybe_data.map_or(Point2D::zero(), |data| { match (*data).flow_construction_result { @@ -708,13 +719,13 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, _ => Point2D::zero() } }); - let property = match *property { - atom!("bottom") => PositionProperty::Bottom, - atom!("top") => PositionProperty::Top, - atom!("left") => PositionProperty::Left, - atom!("right") => PositionProperty::Right, - atom!("width") => PositionProperty::Width, - atom!("height") => PositionProperty::Height, + let property = match longhand_id { + LonghandId::Bottom => PositionProperty::Bottom, + LonghandId::Top => PositionProperty::Top, + LonghandId::Left => PositionProperty::Left, + LonghandId::Right => PositionProperty::Right, + LonghandId::Width => PositionProperty::Width, + LonghandId::Height => PositionProperty::Height, _ => unreachable!() }; let mut iterator = @@ -723,27 +734,25 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, position); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - iterator.result.map(|r| r.to_css_string()) + iterator.result.map(|r| r.to_css_string()).unwrap_or(String::new()) } // TODO: we will return neither the computed nor used value for margin and padding. - // Firefox returns blank strings for the computed value of shorthands, - // so this should be web-compatible. - match *property { - atom!("margin-bottom") | atom!("margin-top") | - atom!("margin-left") | atom!("margin-right") | - atom!("padding-bottom") | atom!("padding-top") | - atom!("padding-left") | atom!("padding-right") + match longhand_id { + LonghandId::MarginBottom | LonghandId::MarginTop | + LonghandId::MarginLeft | LonghandId::MarginRight | + LonghandId::PaddingBottom | LonghandId::PaddingTop | + LonghandId::PaddingLeft | LonghandId::PaddingRight if applies && style.get_box().display != display::computed_value::T::none => { - let (margin_padding, side) = match *property { - atom!("margin-bottom") => (MarginPadding::Margin, Side::Bottom), - atom!("margin-top") => (MarginPadding::Margin, Side::Top), - atom!("margin-left") => (MarginPadding::Margin, Side::Left), - atom!("margin-right") => (MarginPadding::Margin, Side::Right), - atom!("padding-bottom") => (MarginPadding::Padding, Side::Bottom), - atom!("padding-top") => (MarginPadding::Padding, Side::Top), - atom!("padding-left") => (MarginPadding::Padding, Side::Left), - atom!("padding-right") => (MarginPadding::Padding, Side::Right), + let (margin_padding, side) = match longhand_id { + LonghandId::MarginBottom => (MarginPadding::Margin, Side::Bottom), + LonghandId::MarginTop => (MarginPadding::Margin, Side::Top), + LonghandId::MarginLeft => (MarginPadding::Margin, Side::Left), + LonghandId::MarginRight => (MarginPadding::Margin, Side::Right), + LonghandId::PaddingBottom => (MarginPadding::Padding, Side::Bottom), + LonghandId::PaddingTop => (MarginPadding::Padding, Side::Top), + LonghandId::PaddingLeft => (MarginPadding::Padding, Side::Left), + LonghandId::PaddingRight => (MarginPadding::Padding, Side::Right), _ => unreachable!() }; let mut iterator = @@ -753,23 +762,22 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N, style.writing_mode); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - iterator.result.map(|r| r.to_css_string()) + iterator.result.map(|r| r.to_css_string()).unwrap_or(String::new()) }, - atom!("bottom") | atom!("top") | atom!("right") | - atom!("left") + LonghandId::Bottom | LonghandId::Top | LonghandId::Right | LonghandId::Left if applies && positioned && style.get_box().display != display::computed_value::T::none => { - used_value_for_position_property(layout_el, layout_root, requested_node, property) + used_value_for_position_property(layout_el, layout_root, requested_node, longhand_id) } - atom!("width") | atom!("height") + LonghandId::Width | LonghandId::Height if applies && style.get_box().display != display::computed_value::T::none => { - used_value_for_position_property(layout_el, layout_root, requested_node, property) + used_value_for_position_property(layout_el, layout_root, requested_node, longhand_id) } // FIXME: implement used value computation for line-height - ref property => { - style.computed_value_to_string(&*property).ok() + _ => { + style.computed_value_to_string(PropertyDeclarationId::Longhand(longhand_id)) } } } diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 04f35183ff4..bc1d5940f12 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -465,7 +465,7 @@ impl LayoutThread { scroll_root_id_response: None, scroll_area_response: Rect::zero(), overflow_response: NodeOverflowResponse(None), - resolved_style_response: None, + resolved_style_response: String::new(), offset_parent_response: OffsetParentResponse::empty(), margin_style_response: MarginStyleResponse::empty(), stacking_context_scroll_offsets: HashMap::new(), @@ -1017,7 +1017,7 @@ impl LayoutThread { rw_data.scroll_root_id_response = None; }, ReflowQueryType::ResolvedStyleQuery(_, _, _) => { - rw_data.resolved_style_response = None; + rw_data.resolved_style_response = String::new(); }, ReflowQueryType::OffsetParentQuery(_) => { rw_data.offset_parent_response = OffsetParentResponse::empty(); diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index c119f8a70e6..c8be4f560a6 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -12,7 +12,6 @@ use dom::element::Element; use dom::node::{Node, NodeDamage, window_from_node}; use dom::window::Window; use parking_lot::RwLock; -use servo_atoms::Atom; use std::ascii::AsciiExt; use std::sync::Arc; use style::parser::ParserContextExtraData; @@ -74,12 +73,12 @@ impl CSSStyleDeclaration { CSSStyleDeclarationBinding::Wrap) } - fn get_computed_style(&self, property: &Atom) -> Option { + fn get_computed_style(&self, property: PropertyId) -> DOMString { let node = self.owner.upcast::(); if !node.is_in_doc() { // TODO: Node should be matched against the style rules of this window. // Firefox is currently the only browser to implement this. - return None; + return DOMString::new(); } let addr = node.to_trusted_node_address(); window_from_node(&*self.owner).resolved_style_query(addr, self.pseudo.clone(), property) @@ -103,14 +102,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue - fn GetPropertyValue(&self, mut property: DOMString) -> DOMString { - if self.readonly { - // Readonly style declarations are used for getComputedStyle. - property.make_ascii_lowercase(); - let property = Atom::from(property); - return self.get_computed_style(&property).unwrap_or(DOMString::new()); - } - + fn GetPropertyValue(&self, property: DOMString) -> DOMString { let id = if let Ok(id) = PropertyId::parse(property.into()) { id } else { @@ -118,6 +110,11 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { return DOMString::new() }; + if self.readonly { + // Readonly style declarations are used for getComputedStyle. + return self.get_computed_style(id); + } + let style_attribute = self.owner.style_attribute().borrow(); let style_attribute = if let Some(ref lock) = *style_attribute { lock.read() diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 282979dc7bf..db6490b41fd 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -92,6 +92,7 @@ use std::sync::mpsc::TryRecvError::{Disconnected, Empty}; use style::context::ReflowGoal; use style::error_reporting::ParseErrorReporter; use style::media_queries; +use style::properties::PropertyId; use style::properties::longhands::overflow_x; use style::selector_parser::PseudoElement; use style::str::HTML_SPACE_CHARACTERS; @@ -1295,16 +1296,16 @@ impl Window { } pub fn resolved_style_query(&self, - element: TrustedNodeAddress, - pseudo: Option, - property: &Atom) -> Option { + element: TrustedNodeAddress, + pseudo: Option, + property: PropertyId) -> DOMString { if !self.reflow(ReflowGoal::ForScriptQuery, - ReflowQueryType::ResolvedStyleQuery(element, pseudo, property.clone()), + ReflowQueryType::ResolvedStyleQuery(element, pseudo, property), ReflowReason::Query) { - return None; + return DOMString::new(); } let ResolvedStyleResponse(resolved) = self.layout_rpc.resolved_style(); - resolved.map(DOMString::from) + DOMString::from(resolved) } pub fn offset_parent_query(&self, node: TrustedNodeAddress) -> (Option>, Rect) { diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 581adbb45bc..3d0e00c46cc 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -14,11 +14,11 @@ use profile_traits::mem::ReportsChan; use rpc::LayoutRPC; use script_traits::{ConstellationControlMsg, LayoutControlMsg}; use script_traits::{LayoutMsg as ConstellationMsg, StackingContextScrollState, WindowSizeData}; -use servo_atoms::Atom; use servo_url::ServoUrl; use std::sync::Arc; use std::sync::mpsc::{Receiver, Sender}; use style::context::ReflowGoal; +use style::properties::PropertyId; use style::selector_parser::PseudoElement; use style::stylesheets::Stylesheet; @@ -97,7 +97,7 @@ pub enum ReflowQueryType { NodeScrollRootIdQuery(TrustedNodeAddress), NodeGeometryQuery(TrustedNodeAddress), NodeScrollGeometryQuery(TrustedNodeAddress), - ResolvedStyleQuery(TrustedNodeAddress, Option, Atom), + ResolvedStyleQuery(TrustedNodeAddress, Option, PropertyId), OffsetParentQuery(TrustedNodeAddress), MarginStyleQuery(TrustedNodeAddress), } diff --git a/components/script_layout_interface/rpc.rs b/components/script_layout_interface/rpc.rs index 6e19b3f3bcf..7bc58dbf334 100644 --- a/components/script_layout_interface/rpc.rs +++ b/components/script_layout_interface/rpc.rs @@ -57,7 +57,7 @@ pub struct HitTestResponse { pub node_address: Option, } -pub struct ResolvedStyleResponse(pub Option); +pub struct ResolvedStyleResponse(pub String); #[derive(Clone)] pub struct OffsetParentResponse { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 3aa08eaafac..a001c846588 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -599,6 +599,12 @@ pub enum PropertyId { Custom(::custom_properties::Name), } +impl fmt::Debug for PropertyId { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + self.to_css(formatter) + } +} + impl ToCss for PropertyId { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { match *self { @@ -1347,18 +1353,21 @@ impl ComputedValues { false } - pub fn computed_value_to_string(&self, name: &str) -> Result { - match name { + pub fn computed_value_to_string(&self, property: PropertyDeclarationId) -> String { + match property { % for style_struct in data.active_style_structs(): % for longhand in style_struct.longhands: - "${longhand.name}" => Ok(self.${style_struct.ident}.${longhand.ident}.to_css_string()), + PropertyDeclarationId::Longhand(LonghandId::${longhand.camel_case}) => { + self.${style_struct.ident}.${longhand.ident}.to_css_string() + } % endfor % endfor - _ => { - let name = try!(::custom_properties::parse_name(name)); - let map = try!(self.custom_properties.as_ref().ok_or(())); - let value = try!(map.get(&Atom::from(name)).ok_or(())); - Ok(value.to_css_string()) + PropertyDeclarationId::Custom(name) => { + self.custom_properties + .as_ref() + .and_then(|map| map.get(name)) + .map(|value| value.to_css_string()) + .unwrap_or(String::new()) } } } diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 786ce7863f4..98c5b68a2ef 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -101,7 +101,7 @@ fn test_insert() { let rules_list = get_mock_rules(&[".intro.foo", "#top"]); let mut selector_map = SelectorMap::new(); selector_map.insert(rules_list[1][0].clone()); - assert_eq!(1, selector_map.id_hash.get(&atom!("top")).unwrap()[0].source_order); + assert_eq!(1, selector_map.id_hash.get(&Atom::from("top")).unwrap()[0].source_order); selector_map.insert(rules_list[0][0].clone()); assert_eq!(0, selector_map.class_hash.get(&Atom::from("intro")).unwrap()[0].source_order); assert!(selector_map.class_hash.get(&Atom::from("foo")).is_none());