diff options
| -rw-r--r-- | components/script/dom/element.rs | 61 | ||||
| -rw-r--r-- | components/style/properties/properties.mako.rs | 64 | ||||
| -rw-r--r-- | components/style/selector_matching.rs | 12 | ||||
| -rw-r--r-- | tests/unit/style/properties/serialization.rs | 12 | ||||
| -rw-r--r-- | tests/unit/style/stylesheets.rs | 28 |
5 files changed, 107 insertions, 70 deletions
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index f6f8661689c..26ad3eaee40 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -762,12 +762,21 @@ impl Element { 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 index = declarations.declarations - .iter() - .position(|&(ref decl, _)| decl.matches(property)); + let mut importance = None; + let index = declarations.declarations.iter().position(|&(ref decl, i)| { + let matching = decl.matches(property); + if matching { + importance = Some(i) + } + matching + }); if let Some(index) = index { Arc::make_mut(&mut declarations.declarations).remove(index); - declarations.recalc_any(); + if importance.unwrap().important() { + declarations.important_count -= 1; + } else { + declarations.normal_count -= 1; + } } } } @@ -791,21 +800,42 @@ impl Element { '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.normal_count -= 1; + declaration_block.important_count += 1; + } + (Importance::Important, Importance::Normal) => { + declaration_block.normal_count += 1; + 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; + } else { + declaration_block.normal_count += 1; + } } } - declaration_block.recalc_any(); return; } + let (normal_count, important_count) = if importance.important() { + (0, declarations.len() as u32) + } else { + (declarations.len() as u32, 0) + }; + *inline_declarations = Some(PropertyDeclarationBlock { declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()), - any_important: importance.important(), - any_normal: !importance.important(), + normal_count: normal_count, + important_count: important_count, }); } @@ -818,13 +848,24 @@ impl Element { new_importance: Importance) { { let mut inline_declarations = self.style_attribute().borrow_mut(); - if let &mut Some(ref mut declarations) = &mut *inline_declarations { + if let &mut Some(ref mut block) = &mut *inline_declarations { // Usually, the reference counts of `from` and `to` will be 1 here. But transitions // could make them greater than that. - let declarations = Arc::make_mut(&mut declarations.declarations); + let declarations = Arc::make_mut(&mut block.declarations); for &mut (ref declaration, ref mut importance) in declarations { if properties.iter().any(|p| declaration.name() == **p) { - *importance = new_importance + match (*importance, new_importance) { + (Importance::Normal, Importance::Important) => { + block.normal_count -= 1; + block.important_count += 1; + } + (Importance::Important, Importance::Normal) => { + block.normal_count += 1; + block.important_count -= 1; + } + _ => {} + } + *importance = new_importance; } } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 539e03253c4..b0c92710a06 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -288,27 +288,11 @@ pub struct PropertyDeclarationBlock { #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, - /// Whether `self.declaration` contains at least one declaration with `Importance::Normal` - pub any_normal: bool, + /// The number of entries in `self.declaration` with `Importance::Normal` + pub normal_count: u32, - /// Whether `self.declaration` contains at least one declaration with `Importance::Important` - pub any_important: bool, -} - -impl PropertyDeclarationBlock { - pub fn recalc_any(&mut self) { - let mut any_normal = false; - let mut any_important = false; - for &(_, importance) in &*self.declarations { - if importance.important() { - any_important = true - } else { - any_normal = true - } - } - self.any_normal = any_normal; - self.any_important = any_important; - } + /// The number of entries in `self.declaration` with `Importance::Important` + pub important_count: u32, } impl ToCss for PropertyDeclarationBlock { @@ -564,8 +548,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) -> PropertyDeclarationBlock { let mut declarations = Vec::new(); - let mut any_normal = false; - let mut any_important = false; + let mut normal_count = 0; + let mut important_count = 0; let parser = PropertyDeclarationParser { context: context, }; @@ -574,9 +558,9 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars match declaration { Ok((results, importance)) => { if importance.important() { - any_important = true + important_count += results.len() as u32; } else { - any_normal = true + normal_count += results.len() as u32; } declarations.extend(results.into_iter().map(|d| (d, importance))) } @@ -588,50 +572,46 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars } } } - let (declarations, removed_any) = deduplicate_property_declarations(declarations); let mut block = PropertyDeclarationBlock { declarations: Arc::new(declarations), - any_normal: any_normal, - any_important: any_important, + normal_count: normal_count, + important_count: important_count, }; - if removed_any { - block.recalc_any(); - } + deduplicate_property_declarations(&mut block); block } - /// Only keep the "winning" declaration for any given property, by importance then source order. /// The input and output are in source order -fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Importance)>) - -> (Vec<(PropertyDeclaration, Importance)>, bool) { - let mut removed_any = false; +fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) { let mut deduplicated = Vec::new(); let mut seen_normal = PropertyBitField::new(); let mut seen_important = PropertyBitField::new(); let mut seen_custom_normal = Vec::new(); let mut seen_custom_important = Vec::new(); - for (declaration, importance) in declarations.into_iter().rev() { + + let declarations = Arc::get_mut(&mut block.declarations).unwrap(); + for (declaration, importance) in declarations.drain(..).rev() { match declaration { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(..) => { % if not property.derived_from: if importance.important() { if seen_important.get_${property.ident}() { - removed_any = true; + block.important_count -= 1; continue } if seen_normal.get_${property.ident}() { remove_one(&mut deduplicated, |d| { matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _)) }); - removed_any = true; + block.normal_count -= 1; } seen_important.set_${property.ident}() } else { if seen_normal.get_${property.ident}() || seen_important.get_${property.ident}() { - removed_any = true; + block.normal_count -= 1; continue } seen_normal.set_${property.ident}() @@ -644,20 +624,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp PropertyDeclaration::Custom(ref name, _) => { if importance.important() { if seen_custom_important.contains(name) { - removed_any = true; + block.important_count -= 1; continue } if seen_custom_normal.contains(name) { remove_one(&mut deduplicated, |d| { matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name) }); - removed_any = true; + block.normal_count -= 1; } seen_custom_important.push(name.clone()) } else { if seen_custom_normal.contains(name) || seen_custom_important.contains(name) { - removed_any = true; + block.normal_count -= 1; continue } seen_custom_normal.push(name.clone()) @@ -667,7 +647,7 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp deduplicated.push((declaration, importance)) } deduplicated.reverse(); - (deduplicated, removed_any) + *declarations = deduplicated; } #[inline] diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index d04aa322924..781f2298c4a 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -162,8 +162,8 @@ impl Stylist { // Take apart the StyleRule into individual Rules and insert // them into the SelectorMap of that priority. macro_rules! append( - ($style_rule: ident, $priority: ident, $importance: expr, $any: ident) => { - if $style_rule.declarations.$any { + ($style_rule: ident, $priority: ident, $importance: expr, $count: ident) => { + if $style_rule.declarations.$count > 0 { for selector in &$style_rule.selectors { let map = if let Some(ref pseudo) = selector.pseudo_element { self.pseudos_map @@ -191,8 +191,8 @@ impl Stylist { for rule in stylesheet.effective_rules(&self.device) { match *rule { CSSRule::Style(ref style_rule) => { - append!(style_rule, normal, Importance::Normal, any_normal); - append!(style_rule, important, Importance::Important, any_important); + append!(style_rule, normal, Importance::Normal, normal_count); + append!(style_rule, important, Importance::Important, important_count); rules_source_order += 1; for selector in &style_rule.selectors { @@ -394,7 +394,7 @@ impl Stylist { // Step 4: Normal style attributes. if let Some(ref sa) = style_attribute { - if sa.any_normal { + if sa.normal_count > 0 { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, @@ -416,7 +416,7 @@ impl Stylist { // Step 6: `!important` style attributes. if let Some(ref sa) = style_attribute { - if sa.any_important { + if sa.important_count > 0 { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index d22043b237b..f47b5303a0f 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -46,8 +46,10 @@ fn property_declaration_block_should_serialize_correctly() { let block = PropertyDeclarationBlock { declarations: Arc::new(declarations), - any_important: true, - any_normal: true, + + // Incorrect, but not used here: + normal_count: 0, + important_count: 0, }; let css_string = block.to_css_string(); @@ -64,8 +66,10 @@ mod shorthand_serialization { pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String { let block = PropertyDeclarationBlock { declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()), - any_important: false, - any_normal: true, + + // Incorrect, but not used here: + normal_count: 0, + important_count: 0, }; block.to_css_string() diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 28b795d440e..a73f43b51ba 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -24,8 +24,18 @@ fn test_parse_stylesheet() { let css = r" @namespace url(http://www.w3.org/1999/xhtml); /* FIXME: only if scripting is enabled */ - input[type=hidden i] { display: none !important; } - html , body /**/ { display: block; } + input[type=hidden i] { + display: block !important; + display: none !important; + display: inline; + --a: b !important; + --a: inherit !important; + --a: c; + } + html , body /**/ { + display: none; + display: block; + } #d1 > .ok { background: blue; } @keyframes foo { from { width: 0% } @@ -92,9 +102,11 @@ fn test_parse_stylesheet() { (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::none)), Importance::Important), + (PropertyDeclaration::Custom(Atom::from("a"), DeclaredValue::Inherit), + Importance::Important), ]), - any_normal: false, - any_important: true, + normal_count: 0, + important_count: 2, }, }), CSSRule::Style(StyleRule { @@ -140,8 +152,8 @@ fn test_parse_stylesheet() { longhands::display::SpecifiedValue::block)), Importance::Normal), ]), - any_normal: true, - any_important: false, + normal_count: 1, + important_count: 0, }, }), CSSRule::Style(StyleRule { @@ -196,8 +208,8 @@ fn test_parse_stylesheet() { (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial), Importance::Normal), ]), - any_normal: true, - any_important: false, + normal_count: 8, + important_count: 0, }, }), CSSRule::Keyframes(KeyframesRule { |