From bd37f4e694a80773897e94922c381dbd8ad1a41a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 16:27:43 +0200 Subject: [PATCH 1/5] Move CSSStyleDeclaration.GetPropertyValue logic to style --- components/script/dom/cssstyledeclaration.rs | 55 ++++--------------- .../style/properties/declaration_block.rs | 55 ++++++++++++++++--- .../style/properties/properties.mako.rs | 9 --- 3 files changed, 59 insertions(+), 60 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 77aee91ce48..f11cabd7989 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -104,55 +104,24 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue fn GetPropertyValue(&self, mut property: DOMString) -> DOMString { - let owner = &self.owner; - - // Step 1 - property.make_ascii_lowercase(); - let property = Atom::from(property); - 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()); } - // Step 2 - if let Some(shorthand) = Shorthand::from_name(&property) { - let style_attribute = owner.style_attribute().borrow(); - let style_attribute = if let Some(ref style_attribute) = *style_attribute { - style_attribute.read() - } else { - // shorthand.longhands() is never empty, so with no style attribute - // step 2.2.2 would do this: - return DOMString::new() - }; + let style_attribute = self.owner.style_attribute().borrow(); + let style_attribute = if let Some(ref style_attribute) = *style_attribute { + style_attribute.read() + } else { + // No style attribute is like an empty style attribute: no matching declaration. + return DOMString::new() + }; - // Step 2.1 - let mut list = vec![]; - - // Step 2.2 - for longhand in shorthand.longhands() { - // Step 2.2.1 - let declaration = style_attribute.get(longhand); - - // Step 2.2.2 & 2.2.3 - match declaration { - Some(&(ref declaration, _importance)) => list.push(declaration), - None => return DOMString::new(), - } - } - - // Step 2.3 - // TODO: important is hardcoded to false because method does not implement it yet - let serialized_value = shorthand.serialize_shorthand_value_to_string( - list, Importance::Normal); - return DOMString::from(serialized_value); - } - - // Step 3 & 4 - owner.get_inline_style_declaration(&property, |d| match d { - Some(declaration) => DOMString::from(declaration.0.value()), - None => DOMString::new(), - }) + let mut string = String::new(); + style_attribute.property_value_to_css(&property, &mut string).unwrap(); + DOMString::from(string) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index c13a75d2a88..44af1a75ef0 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -6,6 +6,7 @@ use cssparser::{DeclarationListParser, parse_important, ToCss}; use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter}; use error_reporting::ParseErrorReporter; use parser::{ParserContext, ParserContextExtraData, log_css_error}; +use std::ascii::AsciiExt; use std::boxed::Box as StdBox; use std::fmt; use stylesheets::Origin; @@ -66,16 +67,54 @@ impl PropertyDeclarationBlock { pub fn get(&self, property_name: &str) -> Option< &(PropertyDeclaration, Importance)> { self.declarations.iter().find(|&&(ref decl, _)| decl.matches(property_name)) } -} -impl PropertyDeclarationBlock { - // Take a declaration block known to contain a single property, - // and serialize it - pub fn to_css_single_value(&self, dest: &mut W, name: &str) - -> fmt::Result where W: fmt::Write { + /// Find the value of the given property in this block and serialize it + /// + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue + pub fn property_value_to_css(&self, property_name: &str, dest: &mut W) -> fmt::Result + where W: fmt::Write { + // Step 1 + let property = property_name.to_ascii_lowercase(); + + // Step 2 + if let Some(shorthand) = Shorthand::from_name(&property) { + // Step 2.1 + let mut list = Vec::new(); + + // Step 2.2 + for longhand in shorthand.longhands() { + // Step 2.2.1 + let declaration = self.get(longhand); + + // Step 2.2.2 & 2.2.3 + match declaration { + Some(&(ref declaration, _importance)) => list.push(declaration), + None => return Ok(()), + } + } + + // Step 2.3 + // TODO: importance is hardcoded because method does not implement it yet + let importance = Importance::Normal; + let appendable_value = shorthand.get_shorthand_appendable_value(list).unwrap(); + return append_declaration_value(dest, appendable_value, importance) + } + + if let Some(&(ref value, _importance)) = self.get(property_name) { + // Step 3 + value.to_css(dest) + } else { + // Step 4 + Ok(()) + } + } + + /// Take a declaration block known to contain a single property and serialize it. + pub fn single_value_to_css(&self, property_name: &str, dest: &mut W) -> fmt::Result + where W: fmt::Write { match self.declarations.len() { 0 => Err(fmt::Error), - 1 if self.declarations[0].0.name().eq_str_ignore_ascii_case(name) => { + 1 if self.declarations[0].0.name().eq_str_ignore_ascii_case(property_name) => { self.declarations[0].0.to_css(dest) } _ => { @@ -84,7 +123,7 @@ impl PropertyDeclarationBlock { -> &PropertyDeclaration { &dec.0 } - let shorthand = try!(Shorthand::from_name(name).ok_or(fmt::Error)); + let shorthand = try!(Shorthand::from_name(property_name).ok_or(fmt::Error)); if !self.declarations.iter().all(|decl| decl.0.shorthands().contains(&shorthand)) { return Err(fmt::Error) } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 49fa22055df..27bdf9200db 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -410,15 +410,6 @@ impl Shorthand { } } - /// Serializes possible shorthand value to String. - pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, importance: Importance) -> String - where I: IntoIterator, I::IntoIter: Clone { - let appendable_value = self.get_shorthand_appendable_value(declarations).unwrap(); - let mut result = String::new(); - append_declaration_value(&mut result, appendable_value, importance).unwrap(); - result - } - /// Serializes possible shorthand name with value to input buffer given a list of longhand declarations. /// On success, returns true if shorthand value is written and false if no shorthand value is present. pub fn serialize_shorthand_to_buffer<'a, W, I>(self, From fc6a536b3ae52babc17a5a10541afd6208bd241d Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 16:44:29 +0200 Subject: [PATCH 2/5] Move CSSStyleDeclaration.GetPropertyPriority logic to style --- components/script/dom/cssstyledeclaration.rs | 36 ++++++------------- components/script/dom/element.rs | 11 ------ .../style/properties/declaration_block.rs | 21 +++++++++++ 3 files changed, 32 insertions(+), 36 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index f11cabd7989..d347b63b3b0 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -126,34 +126,20 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority fn GetPropertyPriority(&self, mut property: DOMString) -> DOMString { - // Step 1 - property.make_ascii_lowercase(); - let property = Atom::from(property); - - // Step 2 - if let Some(shorthand) = Shorthand::from_name(&property) { - // Step 2.1 & 2.2 & 2.3 - if shorthand.longhands().iter() - .map(|&longhand| self.GetPropertyPriority(DOMString::from(longhand))) - .all(|priority| priority == "important") { - return DOMString::from("important"); - } + let style_attribute = self.owner.style_attribute().borrow(); + let style_attribute = if let Some(ref style_attribute) = *style_attribute { + style_attribute.read() } else { - // Step 3 - return self.owner.get_inline_style_declaration(&property, |d| { - if let Some(decl) = d { - if decl.1.important() { - return DOMString::from("important"); - } - } + // No style attribute is like an empty style attribute: no matching declaration. + return DOMString::new() + }; - // Step 4 - DOMString::new() - }) + if style_attribute.property_priority(&property).important() { + DOMString::from("important") + } else { + // Step 4 + DOMString::new() } - - // Step 4 - DOMString::new() } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 13937bf904c..4c2ac626e7e 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -872,17 +872,6 @@ impl Element { self.sync_property_with_attrs_style(); } - pub fn get_inline_style_declaration(&self, property: &str, f: F) -> R - where F: FnOnce(Option<&(PropertyDeclaration, Importance)>) -> R { - let style_attr = self.style_attribute.borrow(); - if let Some(ref block) = *style_attr { - let block = block.read(); - f(block.get(property)) - } else { - f(None) - } - } - pub fn serialize(&self, traversal_scope: TraversalScope) -> Fallible { let mut writer = vec![]; match serialize(&mut writer, diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 44af1a75ef0..10f4eb42d84 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -109,6 +109,27 @@ impl PropertyDeclarationBlock { } } + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority + pub fn property_priority(&self, property_name: &str) -> Importance { + // Step 1 + let property = property_name.to_ascii_lowercase(); + + // Step 2 + if let Some(shorthand) = Shorthand::from_name(&property) { + // Step 2.1 & 2.2 & 2.3 + if shorthand.longhands().iter().all(|l| { + self.get(l).map_or(false, |&(_, importance)| importance.important()) + }) { + Importance::Important + } else { + Importance::Normal + } + } else { + // Step 3 + self.get(&property).map_or(Importance::Normal, |&(_, importance)| importance) + } + } + /// Take a declaration block known to contain a single property and serialize it. pub fn single_value_to_css(&self, property_name: &str, dest: &mut W) -> fmt::Result where W: fmt::Write { From bd4a4c38c86bb7275c24718686ee3a7e65dc1933 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 17:06:56 +0200 Subject: [PATCH 3/5] Move CSSStyleDeclaration.RemoveProperty logic to style --- components/script/dom/cssstyledeclaration.rs | 50 ++++++++++--------- components/script/dom/element.rs | 31 ++---------- .../style/properties/declaration_block.rs | 24 +++++++++ 3 files changed, 55 insertions(+), 50 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index d347b63b3b0..81ab5090385 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -125,7 +125,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority - fn GetPropertyPriority(&self, mut property: DOMString) -> DOMString { + fn GetPropertyPriority(&self, property: DOMString) -> DOMString { let style_attribute = self.owner.style_attribute().borrow(); let style_attribute = if let Some(ref style_attribute) = *style_attribute { style_attribute.read() @@ -237,36 +237,40 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty - fn RemoveProperty(&self, mut property: DOMString) -> Fallible { + fn RemoveProperty(&self, property: DOMString) -> Fallible { // Step 1 if self.readonly { return Err(Error::NoModificationAllowed); } - // Step 2 - property.make_ascii_lowercase(); + let mut style_attribute = self.owner.style_attribute().borrow_mut(); + let mut string = String::new(); + let empty; + { + let mut style_attribute = if let Some(ref mut style_attribute) = *style_attribute { + style_attribute.write() + } else { + // No style attribute is like an empty style attribute: nothing to remove. + return Ok(DOMString::new()) + }; - // Step 3 - let value = self.GetPropertyValue(property.clone()); + // Step 3 + style_attribute.property_value_to_css(&property, &mut string).unwrap(); - let element = self.owner.upcast::(); - - match Shorthand::from_name(&property) { - // Step 4 - Some(shorthand) => { - for longhand in shorthand.longhands() { - element.remove_inline_style_property(longhand) - } - } - // Step 5 - None => element.remove_inline_style_property(&property), + // Step 4 & 5 + style_attribute.remove_property(&property); + self.owner.set_style_attr(style_attribute.to_css_string()); + empty = style_attribute.declarations.is_empty() + } + if empty { + *style_attribute = None; } - let node = element.upcast::(); + let node = self.owner.upcast::(); node.dirty(NodeDamage::NodeStyleDamaged); // Step 6 - Ok(value) + Ok(DOMString::from(string)) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat @@ -311,7 +315,6 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext fn SetCssText(&self, value: DOMString) -> ErrorResult { let window = window_from_node(self.owner.upcast::()); - let element = self.owner.upcast::(); // Step 1 if self.readonly { @@ -321,13 +324,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 3 let decl_block = parse_style_attribute(&value, &window.get_url(), window.css_error_reporter(), ParserContextExtraData::default()); - *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { + *self.owner.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { + self.owner.set_style_attr(String::new()); None // Step 2 } else { + self.owner.set_style_attr(decl_block.to_css_string()); Some(Arc::new(RwLock::new(decl_block))) }; - element.sync_property_with_attrs_style(); - let node = element.upcast::(); + let node = self.owner.upcast::(); node.dirty(NodeDamage::NodeStyleDamaged); Ok(()) } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 4c2ac626e7e..eba2d2cd370 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -741,8 +741,11 @@ impl Element { } else { String::new() }; + self.set_style_attr(style_str) + } - let mut new_style = AttrValue::String(style_str); + pub fn set_style_attr(&self, new_value: String) { + let mut new_style = AttrValue::String(new_value); if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &atom!("style")) { style_attr.swap_value(&mut new_style); @@ -764,32 +767,6 @@ impl Element { self.attrs.borrow_mut().push(JS::from_ref(&attr)); } - pub fn remove_inline_style_property(&self, property: &str) { - fn remove(element: &Element, property: &str) { - let mut inline_declarations = element.style_attribute.borrow_mut(); - if let &mut Some(ref mut declarations) = &mut *inline_declarations { - let mut importance = None; - let index = declarations.read().declarations.iter().position(|&(ref decl, i)| { - let matching = decl.matches(property); - if matching { - importance = Some(i) - } - matching - }); - if let Some(index) = index { - let mut declarations = declarations.write(); - declarations.declarations.remove(index); - if importance.unwrap().important() { - declarations.important_count -= 1; - } - } - } - } - - remove(self, property); - self.sync_property_with_attrs_style(); - } - pub fn update_inline_style(&self, declarations: Vec, importance: Importance) { diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 10f4eb42d84..ec0412b544b 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -130,6 +130,30 @@ impl PropertyDeclarationBlock { } } + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty + pub fn remove_property(&mut self, property_name: &str) { + // Step 2 + let property = property_name.to_ascii_lowercase(); + + match Shorthand::from_name(&property) { + // Step 4 + Some(shorthand) => self.remove_longhands(shorthand.longhands()), + // Step 5 + None => self.remove_longhands(&[&*property]), + } + } + + fn remove_longhands(&mut self, names: &[&str]) { + let important_count = &mut self.important_count; + self.declarations.retain(|&(ref declaration, importance)| { + let retain = !names.iter().any(|n| declaration.matches(n)); + if !retain && importance.important() { + *important_count -= 1 + } + retain + }) + } + /// Take a declaration block known to contain a single property and serialize it. pub fn single_value_to_css(&self, property_name: &str, dest: &mut W) -> fmt::Result where W: fmt::Write { From c740a76410f4ff0c0972f3ea55f593527d244be7 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 18:44:05 +0200 Subject: [PATCH 4/5] Move (some) CSSStyleDeclaration.SetProperty logic to style --- components/script/dom/cssstyledeclaration.rs | 122 +++++++++++------- components/script/dom/element.rs | 53 -------- .../style/properties/declaration_block.rs | 24 ++++ .../style/properties/properties.mako.rs | 2 + 4 files changed, 103 insertions(+), 98 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 81ab5090385..abd6d207dd1 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -17,7 +17,7 @@ use std::ascii::AsciiExt; use std::sync::Arc; use string_cache::Atom; use style::parser::ParserContextExtraData; -use style::properties::{Shorthand, Importance}; +use style::properties::{Shorthand, Importance, PropertyDeclarationBlock}; use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute}; use style::selector_impl::PseudoElement; @@ -91,7 +91,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { fn Length(&self) -> u32 { let elem = self.owner.upcast::(); let len = match *elem.style_attribute().borrow() { - Some(ref declarations) => declarations.read().declarations.len(), + Some(ref lock) => lock.read().declarations.len(), None => 0, }; len as u32 @@ -112,8 +112,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } let style_attribute = self.owner.style_attribute().borrow(); - let style_attribute = if let Some(ref style_attribute) = *style_attribute { - style_attribute.read() + let style_attribute = if let Some(ref lock) = *style_attribute { + lock.read() } else { // No style attribute is like an empty style attribute: no matching declaration. return DOMString::new() @@ -127,8 +127,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority fn GetPropertyPriority(&self, property: DOMString) -> DOMString { let style_attribute = self.owner.style_attribute().borrow(); - let style_attribute = if let Some(ref style_attribute) = *style_attribute { - style_attribute.read() + let style_attribute = if let Some(ref lock) = *style_attribute { + lock.read() } else { // No style attribute is like an empty style attribute: no matching declaration. return DOMString::new() @@ -144,7 +144,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty fn SetProperty(&self, - mut property: DOMString, + property: DOMString, value: DOMString, priority: DOMString) -> ErrorResult { @@ -153,46 +153,78 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { return Err(Error::NoModificationAllowed); } - // Step 2 - property.make_ascii_lowercase(); - // Step 3 if !is_supported_property(&property) { return Ok(()); } - // Step 4 + let mut style_attribute = self.owner.style_attribute().borrow_mut(); + if value.is_empty() { - return self.RemoveProperty(property).map(|_| ()); + // Step 4 + let empty; + { + let mut style_attribute = if let Some(ref lock) = *style_attribute { + lock.write() + } else { + // No style attribute is like an empty style attribute: nothing to remove. + return Ok(()) + }; + + style_attribute.remove_property(&property); + empty = style_attribute.declarations.is_empty() + } + if empty { + *style_attribute = None; + } + } else { + // Step 5 + let importance = match &*priority { + "" => Importance::Normal, + p if p.eq_ignore_ascii_case("important") => Importance::Important, + _ => return Ok(()), + }; + + // Step 6 + let window = window_from_node(&*self.owner); + let declarations = + parse_one_declaration(&property, &value, &window.get_url(), window.css_error_reporter(), + ParserContextExtraData::default()); + + // Step 7 + let declarations = if let Ok(declarations) = declarations { + declarations + } else { + return Ok(()); + }; + + // Step 8 + // Step 9 + match *style_attribute { + Some(ref lock) => { + let mut style_attribute = lock.write(); + for declaration in declarations { + style_attribute.set_parsed_declaration(declaration, importance); + } + self.owner.set_style_attr(style_attribute.to_css_string()); + } + ref mut option @ None => { + let important_count = if importance.important() { + declarations.len() as u32 + } else { + 0 + }; + let block = PropertyDeclarationBlock { + declarations: declarations.into_iter().map(|d| (d, importance)).collect(), + important_count: important_count, + }; + self.owner.set_style_attr(block.to_css_string()); + *option = Some(Arc::new(RwLock::new(block))); + } + } } - // Step 5 - let priority = match &*priority { - "" => Importance::Normal, - p if p.eq_ignore_ascii_case("important") => Importance::Important, - _ => return Ok(()), - }; - - // Step 6 - let window = window_from_node(&*self.owner); - let declarations = - parse_one_declaration(&property, &value, &window.get_url(), window.css_error_reporter(), - ParserContextExtraData::default()); - - // Step 7 - let declarations = if let Ok(declarations) = declarations { - declarations - } else { - return Ok(()); - }; - - let element = self.owner.upcast::(); - - // Step 8 - // Step 9 - element.update_inline_style(declarations, priority); - - let node = element.upcast::(); + let node = self.owner.upcast::(); node.dirty(NodeDamage::NodeStyleDamaged); Ok(()) } @@ -247,8 +279,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let mut string = String::new(); let empty; { - let mut style_attribute = if let Some(ref mut style_attribute) = *style_attribute { - style_attribute.write() + let mut style_attribute = if let Some(ref lock) = *style_attribute { + lock.write() } else { // No style attribute is like an empty style attribute: nothing to remove. return Ok(DOMString::new()) @@ -288,8 +320,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let index = index as usize; let elem = self.owner.upcast::(); let style_attribute = elem.style_attribute().borrow(); - style_attribute.as_ref().and_then(|declarations| { - declarations.read().declarations.get(index).map(|entry| { + style_attribute.as_ref().and_then(|lock| { + lock.read().declarations.get(index).map(|entry| { let (ref declaration, importance) = *entry; let mut css = declaration.to_css_string(); if importance.important() { @@ -305,8 +337,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let elem = self.owner.upcast::(); let style_attribute = elem.style_attribute().borrow(); - if let Some(declarations) = style_attribute.as_ref() { - DOMString::from(declarations.read().to_css_string()) + if let Some(lock) = style_attribute.as_ref() { + DOMString::from(lock.read().to_css_string()) } else { DOMString::new() } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index eba2d2cd370..67e0b549d56 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -767,59 +767,6 @@ impl Element { self.attrs.borrow_mut().push(JS::from_ref(&attr)); } - pub fn update_inline_style(&self, - declarations: Vec, - importance: Importance) { - fn update(element: &Element, declarations: Vec, - importance: Importance) { - let mut inline_declarations = element.style_attribute().borrow_mut(); - if let &mut Some(ref mut declaration_block) = &mut *inline_declarations { - { - let mut declaration_block = declaration_block.write(); - let declaration_block = &mut *declaration_block; - let existing_declarations = &mut declaration_block.declarations; - - 'outer: for incoming_declaration in declarations { - for existing_declaration in &mut *existing_declarations { - if existing_declaration.0.name() == incoming_declaration.name() { - match (existing_declaration.1, importance) { - (Importance::Normal, Importance::Important) => { - declaration_block.important_count += 1; - } - (Importance::Important, Importance::Normal) => { - declaration_block.important_count -= 1; - } - _ => {} - } - *existing_declaration = (incoming_declaration, importance); - continue 'outer; - } - } - existing_declarations.push((incoming_declaration, importance)); - if importance.important() { - declaration_block.important_count += 1; - } - } - } - return; - } - - let important_count = if importance.important() { - declarations.len() as u32 - } else { - 0 - }; - - *inline_declarations = Some(Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: declarations.into_iter().map(|d| (d, importance)).collect(), - important_count: important_count, - }))); - } - - update(self, declarations, importance); - self.sync_property_with_attrs_style(); - } - pub fn set_inline_style_property_priority(&self, properties: &[&str], new_importance: Importance) { diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index ec0412b544b..2300634b559 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -130,6 +130,30 @@ impl PropertyDeclarationBlock { } } + pub fn set_parsed_declaration(&mut self, declaration: PropertyDeclaration, + importance: Importance) { + for slot in &mut *self.declarations { + if slot.0.name() == declaration.name() { + match (slot.1, importance) { + (Importance::Normal, Importance::Important) => { + self.important_count += 1; + } + (Importance::Important, Importance::Normal) => { + self.important_count -= 1; + } + _ => {} + } + *slot = (declaration, importance); + return + } + } + + self.declarations.push((declaration, importance)); + if importance.important() { + self.important_count += 1; + } + } + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty pub fn remove_property(&mut self, property_name: &str) { // Step 2 diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 27bdf9200db..d4da49022d5 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1974,6 +1974,8 @@ pub fn modify_style_for_inline_absolute_hypothetical_fragment(style: &mut Arc bool { match_ignore_ascii_case! { property, % for property in data.shorthands + data.longhands: From 0eed39c19824c73cede4ab37ab145dcf721f6426 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 19:20:05 +0200 Subject: [PATCH 5/5] Move CSSStyleDeclaration.SetPropertyPriority logic to style --- components/script/dom/cssstyledeclaration.rs | 22 +++++----- components/script/dom/element.rs | 40 +------------------ .../style/properties/declaration_block.rs | 17 ++++++++ 3 files changed, 30 insertions(+), 49 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index abd6d207dd1..37ff6e7a3bf 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -242,24 +242,26 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // Step 4 - let priority = match &*priority { + let importance = match &*priority { "" => Importance::Normal, p if p.eq_ignore_ascii_case("important") => Importance::Important, _ => return Ok(()), }; - let element = self.owner.upcast::(); + let style_attribute = self.owner.style_attribute().borrow(); + if let Some(ref lock) = *style_attribute { + let mut style_attribute = lock.write(); - // Step 5 & 6 - match Shorthand::from_name(&property) { - Some(shorthand) => { - element.set_inline_style_property_priority(shorthand.longhands(), priority) + // Step 5 & 6 + match Shorthand::from_name(&property) { + Some(shorthand) => style_attribute.set_importance(shorthand.longhands(), importance), + None => style_attribute.set_importance(&[&*property], importance), } - None => element.set_inline_style_property_priority(&[&*property], priority), - } - let node = element.upcast::(); - node.dirty(NodeDamage::NodeStyleDamaged); + self.owner.set_style_attr(style_attribute.to_css_string()); + let node = self.owner.upcast::(); + node.dirty(NodeDamage::NodeStyleDamaged); + } Ok(()) } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 67e0b549d56..621faa912aa 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -5,7 +5,7 @@ //! Element nodes. use app_units::Au; -use cssparser::{Color, ToCss}; +use cssparser::Color; use devtools_traits::AttrInfo; use dom::activation::Activatable; use dom::attr::{Attr, AttrHelpersForLayout}; @@ -735,15 +735,6 @@ impl Element { // this sync method is called upon modification of the style_attribute property, // therefore, it should not trigger subsequent mutation events - pub fn sync_property_with_attrs_style(&self) { - let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() { - declarations.read().to_css_string() - } else { - String::new() - }; - self.set_style_attr(style_str) - } - pub fn set_style_attr(&self, new_value: String) { let mut new_style = AttrValue::String(new_value); @@ -767,35 +758,6 @@ impl Element { self.attrs.borrow_mut().push(JS::from_ref(&attr)); } - pub fn set_inline_style_property_priority(&self, - properties: &[&str], - new_importance: Importance) { - { - let mut inline_declarations = self.style_attribute().borrow_mut(); - if let &mut Some(ref mut block) = &mut *inline_declarations { - let mut block = block.write(); - let block = &mut *block; - let declarations = &mut block.declarations; - for &mut (ref declaration, ref mut importance) in declarations { - if properties.iter().any(|p| declaration.name() == **p) { - match (*importance, new_importance) { - (Importance::Normal, Importance::Important) => { - block.important_count += 1; - } - (Importance::Important, Importance::Normal) => { - block.important_count -= 1; - } - _ => {} - } - *importance = new_importance; - } - } - } - } - - self.sync_property_with_attrs_style(); - } - pub fn serialize(&self, traversal_scope: TraversalScope) -> Fallible { let mut writer = vec![]; match serialize(&mut writer, diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 2300634b559..25d48a1cb55 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -154,6 +154,23 @@ impl PropertyDeclarationBlock { } } + pub fn set_importance(&mut self, property_names: &[&str], new_importance: Importance) { + for &mut (ref declaration, ref mut importance) in &mut self.declarations { + if property_names.iter().any(|p| declaration.matches(p)) { + match (*importance, new_importance) { + (Importance::Normal, Importance::Important) => { + self.important_count += 1; + } + (Importance::Important, Importance::Normal) => { + self.important_count -= 1; + } + _ => {} + } + *importance = new_importance; + } + } + } + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty pub fn remove_property(&mut self, property_name: &str) { // Step 2