diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index d8ac84a8ec5..bd0a87f0b0a 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -930,7 +930,7 @@ impl PropertyDeclarationBlock { let mut already_serialized = NonCustomPropertyIdSet::new(); // Step 3 - for (declaration, importance) in self.declaration_importance_iter() { + 'declaration_loop: for (declaration, importance) in self.declaration_importance_iter() { // Step 3.1 let property = declaration.id(); let longhand_id = match property { @@ -956,11 +956,7 @@ impl PropertyDeclarationBlock { continue; } - // Step 3.3 - // Step 3.3.1 is done by checking already_serialized while - // iterating below. - - // Step 3.3.2 + // Steps 3.3 & 3.4 for shorthand in longhand_id.shorthands() { // We already attempted to serialize this shorthand before. if already_serialized.contains(shorthand.into()) { @@ -972,74 +968,105 @@ impl PropertyDeclarationBlock { continue; } - // Substep 2 & 3 - let mut current_longhands = SmallVec::<[_; 10]>::new(); - let mut important_count = 0; - let mut found_system = None; - - let is_system_font = shorthand == ShorthandId::Font && - self.declarations.iter().any(|l| match l.id() { - PropertyDeclarationId::Longhand(id) => { - if already_serialized.contains(id.into()) { - return false; - } - - l.get_system().is_some() - }, - PropertyDeclarationId::Custom(..) => { - debug_assert!(l.get_system().is_none()); - false - }, - }); - - if is_system_font { - for (longhand, importance) in self.declaration_importance_iter() { - if longhand.get_system().is_some() || longhand.is_default_line_height() { - current_longhands.push(longhand); - if found_system.is_none() { - found_system = longhand.get_system(); - } - if importance.important() { - important_count += 1; - } - } - } - } else { - let mut contains_all_longhands = true; + // Step 3.3.1: + // Let longhands be an array consisting of all CSS + // declarations in declaration block’s declarations that + // that are not in already serialized and have a property + // name that maps to one of the shorthand properties in + // shorthands. + let longhands = { + // TODO(emilio): This could just index in an array if we + // remove pref-controlled longhands. + let mut ids = LonghandIdSet::new(); for longhand in shorthand.longhands() { - match self.get(PropertyDeclarationId::Longhand(longhand)) { - Some((declaration, importance)) => { - current_longhands.push(declaration); - if importance.important() { - important_count += 1; - } - }, - None => { - contains_all_longhands = false; - break; - }, - } + ids.insert(longhand); } + ids + }; - // Substep 1: - if !contains_all_longhands { - continue; + // Step 3.4.2 + // If all properties that map to shorthand are not present + // in longhands, continue with the steps labeled shorthand + // loop. + if !self.longhands.contains_all(&longhands) { + continue; + } + + // Step 3.4.3: + // Let current longhands be an empty array. + let mut current_longhands = SmallVec::<[_; 10]>::new(); + let mut logical_groups = LogicalGroupSet::new(); + let mut saw_one = false; + let mut logical_mismatch = false; + let mut seen = LonghandIdSet::new(); + let mut important_count = 0; + + // Step 3.4.4: + // Append all CSS declarations in longhands that have a + // property name that maps to shorthand to current longhands. + for (declaration, importance) in self.declaration_importance_iter() { + let longhand = match declaration.id() { + PropertyDeclarationId::Longhand(id) => id, + PropertyDeclarationId::Custom(..) => continue, + }; + + if longhands.contains(longhand) { + saw_one = true; + if importance.important() { + important_count += 1; + } + current_longhands.push(declaration); + if shorthand != ShorthandId::All { + // All is special because it contains both physical + // and logical longhands. + if let Some(g) = longhand.logical_group() { + logical_groups.insert(g); + } + seen.insert(longhand); + if seen == longhands { + break; + } + } + } else if saw_one { + if let Some(g) = longhand.logical_group() { + if logical_groups.contains(g) { + logical_mismatch = true; + break; + } + } } } - // Substep 4 + // 3.4.5: + // If there is one or more CSS declarations in current + // longhands have their important flag set and one or more + // with it unset, continue with the steps labeled shorthand + // loop. let is_important = important_count > 0; if is_important && important_count != current_longhands.len() { continue; } + + // 3.4.6: + // If there’s any declaration in declaration block in between + // the first and the last longhand in current longhands which + // belongs to the same logical property group, but has a + // different mapping logic as any of the longhands in current + // longhands, and is not in current longhands, continue with + // the steps labeled shorthand loop. + if logical_mismatch { + continue; + } + let importance = if is_important { Importance::Important } else { Importance::Normal }; - // Substep 5 - Let value be the result of invoking serialize - // a CSS value of current longhands. + // 3.4.7: + // Let value be the result of invoking serialize a CSS value + // of current longhands. let appendable_value = match shorthand .get_shorthand_appendable_value(current_longhands.iter().cloned()) { @@ -1050,32 +1077,17 @@ impl PropertyDeclarationBlock { // We avoid re-serializing if we're already an // AppendableValue::Css. let mut v = CssString::new(); - let value = match (appendable_value, found_system) { - ( - AppendableValue::Css { - css, - with_variables, - }, - _, - ) => { + let value = match appendable_value { + AppendableValue::Css { css, with_variables } => { debug_assert!(!css.is_empty()); - AppendableValue::Css { - css: css, - with_variables: with_variables, - } + AppendableValue::Css { css, with_variables } }, - #[cfg(feature = "gecko")] - (_, Some(sys)) => { - sys.to_css(&mut CssWriter::new(&mut v))?; - AppendableValue::Css { - css: CssStringBorrow::from(&v), - with_variables: false, - } - }, - (other, _) => { + other => { append_declaration_value(&mut v, other)?; - // Substep 6 + // 3.4.8: + // If value is the empty string, continue with the + // steps labeled shorthand loop. if v.is_empty() { continue; } @@ -1087,7 +1099,15 @@ impl PropertyDeclarationBlock { }, }; - // Substeps 7 and 8 + // 3.4.9: + // Let serialized declaration be the result of invoking + // serialize a CSS declaration with property name shorthand, + // value value, and the important flag set if the CSS + // declarations in current longhands have their important + // flag set. + // + // 3.4.10: + // Append serialized declaration to list. append_serialization::>, _>( dest, &shorthand, @@ -1096,6 +1116,9 @@ impl PropertyDeclarationBlock { &mut is_first_serialization, )?; + // 3.4.11: + // Append the property names of all items of current + // longhands to already serialized. for current_longhand in ¤t_longhands { let longhand_id = match current_longhand.id() { PropertyDeclarationId::Longhand(id) => id, @@ -1106,24 +1129,21 @@ impl PropertyDeclarationBlock { already_serialized.insert(longhand_id.into()); } - // FIXME(https://github.com/w3c/csswg-drafts/issues/1774) - // The specification does not include an instruction to abort - // the shorthand loop at this point, but doing so both matches - // Gecko and makes sense since shorthands are checked in - // preferred order. - break; + // 3.4.12: + // Continue with the steps labeled declaration loop. + continue 'declaration_loop; } - // Step 3.3.4 - if already_serialized.contains(longhand_id.into()) { - continue; - } - - // Steps 3.3.5, 3.3.6 & 3.3.7 - // Need to specify an iterator type here even though it’s unused to work around - // "error: unable to infer enough type information about `_`; - // type annotations or generic parameter binding required [E0282]" - // Use the same type as earlier call to reuse generated code. + // Steps 3.5, 3.6 & 3.7: + // Let value be the result of invoking serialize a CSS value of + // declaration. + // + // Let serialized declaration be the result of invoking + // serialize a CSS declaration with property name property, + // value value, and the important flag set if declaration has + // its important flag set. + // + // Append serialized declaration to list. append_serialization::>, _>( dest, &property, @@ -1132,7 +1152,8 @@ impl PropertyDeclarationBlock { &mut is_first_serialization, )?; - // Step 3.3.8 + // Step 3.8: + // Append property to already serialized. already_serialized.insert(longhand_id.into()); } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 24d396cdf0f..a4316a501ad 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -792,14 +792,44 @@ static ${name}: LonghandIdSet = LonghandIdSet { /// A group for properties which may override each other /// via logical resolution. #[derive(Clone, Copy, Eq, Hash, PartialEq)] +#[repr(u8)] pub enum LogicalGroup { - % for group in sorted(logical_groups.keys()): + % for i, group in enumerate(logical_groups.keys()): /// ${group} - ${to_camel_case(group)}, + ${to_camel_case(group)} = ${i}, % endfor } +/// A set of logical groups. +#[derive(Clone, Copy, Debug, Default, MallocSizeOf, PartialEq)] +pub struct LogicalGroupSet { + storage: [u32; (${len(logical_groups)} - 1 + 32) / 32] +} + +impl LogicalGroupSet { + /// Creates an empty `NonCustomPropertyIdSet`. + pub fn new() -> Self { + Self { + storage: Default::default(), + } + } + + /// Return whether the given group is in the set + #[inline] + pub fn contains(&self, g: LogicalGroup) -> bool { + let bit = g as usize; + (self.storage[bit / 32] & (1 << (bit % 32))) != 0 + } + + /// Insert a group the set. + #[inline] + pub fn insert(&mut self, g: LogicalGroup) { + let bit = g as usize; + self.storage[bit / 32] |= 1 << (bit % 32); + } +} + /// A set of longhand properties #[derive(Clone, Copy, Debug, Default, MallocSizeOf, PartialEq)] pub struct LonghandIdSet {