diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 055d33a62c9..cae2495be2e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1371,10 +1371,11 @@ fn static_assert() { let ident = v.ident.as_ref().map_or(&[] as &[_], |ident| ident.0.as_slice()); self.gecko.${value.gecko}.mLineName.assign(ident); self.gecko.${value.gecko}.mHasSpan = v.is_span; - self.gecko.${value.gecko}.mInteger = v.line_num.map(|i| { + if let Some(integer) = v.line_num { // clamping the integer between a range - cmp::max(nsStyleGridLine_kMinLine, cmp::min(i.value(), nsStyleGridLine_kMaxLine)) - }).unwrap_or(0); + self.gecko.${value.gecko}.mInteger = cmp::max(nsStyleGridLine_kMinLine, + cmp::min(integer.value(), nsStyleGridLine_kMaxLine)); + } } pub fn copy_${value.name}_from(&mut self, other: &Self) { @@ -1520,7 +1521,7 @@ fn static_assert() { &mut ${self_grid}.mRepeatAutoLineNameListAfter, 0); } }, - GridTemplateComponent::Subgrid(mut list) => { + GridTemplateComponent::Subgrid(list) => { ${self_grid}.set_mIsSubgrid(true); let names_length = match list.fill_idx { Some(_) => list.names.len() - 1, @@ -1537,14 +1538,15 @@ fn static_assert() { &mut ${self_grid}.mRepeatAutoLineNameListAfter, 0); } + let mut names = list.names.into_vec(); if let Some(idx) = list.fill_idx { ${self_grid}.set_mIsAutoFill(true); ${self_grid}.mRepeatAutoIndex = idx as i16; - set_line_names(&list.names.swap_remove(idx as usize), + set_line_names(&names.swap_remove(idx as usize), &mut ${self_grid}.mRepeatAutoLineNameListBefore); } - for (servo_names, gecko_names) in list.names.iter().zip(${self_grid}.mLineNameLists.iter_mut()) { + for (servo_names, gecko_names) in names.iter().zip(${self_grid}.mLineNameLists.iter_mut()) { set_line_names(servo_names, gecko_names); } }, diff --git a/components/style/properties/shorthand/position.mako.rs b/components/style/properties/shorthand/position.mako.rs index af4f218ee44..e49c39bc7b8 100644 --- a/components/style/properties/shorthand/position.mako.rs +++ b/components/style/properties/shorthand/position.mako.rs @@ -252,7 +252,6 @@ -> Result<(GridTemplateComponent, GridTemplateComponent, Either), ParseError<'i>> { - // Other shorthand sub properties also parse `none` and `subgrid` keywords and this // shorthand should know after these keywords there is nothing to parse. Otherwise it // gets confused and rejects the sub properties that contains `none` or `subgrid`. @@ -278,34 +277,35 @@ } % endfor - let first_line_names = input.try(parse_line_names).unwrap_or(vec![]); + let first_line_names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()); if let Ok(s) = input.try(Parser::expect_string) { let mut strings = vec![]; let mut values = vec![]; let mut line_names = vec![]; - let mut names = first_line_names; + let mut names = first_line_names.into_vec(); let mut string = s.into_owned().into_boxed_str(); loop { - line_names.push(names); + line_names.push(names.into_boxed_slice()); strings.push(string); let size = input.try(|i| TrackSize::parse(context, i)).unwrap_or_default(); values.push(size); - names = input.try(parse_line_names).unwrap_or(vec![]); + names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()).into_vec(); if let Ok(v) = input.try(parse_line_names) { - names.extend(v); + names.extend(v.into_vec()); } string = match input.try(Parser::expect_string) { Ok(s) => s.into_owned().into_boxed_str(), _ => { // only the named area determines whether we should bail out - line_names.push(names); + line_names.push(names.into_boxed_slice()); break }, }; } if line_names.len() == values.len() { - line_names.push(vec![]); // should be one longer than track sizes + // should be one longer than track sizes + line_names.push(vec![].into_boxed_slice()); } let template_areas = TemplateAreas::from_vec(strings) @@ -313,7 +313,7 @@ let template_rows = TrackList { list_type: TrackListType::Normal, values: values, - line_names: line_names, + line_names: line_names.into_boxed_slice(), auto_repeat: None, }; @@ -372,11 +372,41 @@ template_columns.to_css(dest) }, Either::First(ref areas) => { + // The length of template-area and template-rows values should be equal. + if areas.strings.len() != template_rows.track_list_len() { + return Ok(()); + } + let track_list = match *template_rows { - GenericGridTemplateComponent::TrackList(ref list) => list, - _ => unreachable!(), // should exist! + GenericGridTemplateComponent::TrackList(ref list) => { + // We should fail if there is a `repeat` function. `grid` and + // `grid-template` shorthands doesn't accept that. Only longhand accepts. + if list.auto_repeat.is_some() { + return Ok(()); + } + list + }, + // Others template components shouldn't exist with normal shorthand values. + // But if we need to serialize a group of longhand sub-properties for + // the shorthand, we should be able to return empty string instead of crashing. + _ => return Ok(()), }; + // We need to check some values that longhand accepts but shorthands don't. + match *template_columns { + // We should fail if there is a `repeat` function. `grid` and + // `grid-template` shorthands doesn't accept that. Only longhand accepts that. + GenericGridTemplateComponent::TrackList(ref list) if list.auto_repeat.is_some() => { + return Ok(()); + }, + // Also the shorthands don't accept subgrids unlike longhand. + // We should fail without an error here. + GenericGridTemplateComponent::Subgrid(_) => { + return Ok(()); + }, + _ => {}, + } + let mut names_iter = track_list.line_names.iter(); for (((i, string), names), size) in areas.strings.iter().enumerate() .zip(&mut names_iter) @@ -428,8 +458,8 @@ use properties::longhands::{grid_auto_columns, grid_auto_rows, grid_auto_flow}; use properties::longhands::grid_auto_flow::computed_value::{AutoFlow, T as SpecifiedAutoFlow}; use values::{Either, None_}; - use values::generics::grid::GridTemplateComponent; - use values::specified::{LengthOrPercentage, TrackSize}; + use values::generics::grid::{GridTemplateComponent, TrackListType}; + use values::specified::{GenericGridTemplateComponent, LengthOrPercentage, TrackSize}; pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { @@ -507,6 +537,14 @@ impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + // `grid` shorthand resets these properties. If they are not zero, that means they + // are changed by longhands and in that case we should fail serializing `grid`. + if *self.grid_row_gap != LengthOrPercentage::zero() || + *self.grid_column_gap != LengthOrPercentage::zero() { + return Ok(()); + } + + if *self.grid_template_areas != Either::Second(None_) || (*self.grid_template_rows != GridTemplateComponent::None && *self.grid_template_columns != GridTemplateComponent::None) || @@ -517,6 +555,19 @@ } if self.grid_auto_flow.autoflow == AutoFlow::Column { + // It should fail to serialize if other branch of the if condition's values are set. + if *self.grid_auto_rows != TrackSize::default() || + *self.grid_template_columns != GridTemplateComponent::None { + return Ok(()); + } + + // It should fail to serialize if template-rows value is not Explicit. + if let GenericGridTemplateComponent::TrackList(ref list) = *self.grid_template_rows { + if list.list_type != TrackListType::Explicit { + return Ok(()); + } + } + self.grid_template_rows.to_css(dest)?; dest.write_str(" / auto-flow")?; if self.grid_auto_flow.dense { @@ -528,6 +579,19 @@ self.grid_auto_columns.to_css(dest)?; } } else { + // It should fail to serialize if other branch of the if condition's values are set. + if *self.grid_auto_columns != TrackSize::default() || + *self.grid_template_rows != GridTemplateComponent::None { + return Ok(()); + } + + // It should fail to serialize if template-column value is not Explicit. + if let GenericGridTemplateComponent::TrackList(ref list) = *self.grid_template_columns { + if list.list_type != TrackListType::Explicit { + return Ok(()); + } + } + dest.write_str("auto-flow")?; if self.grid_auto_flow.dense { dest.write_str(" dense")?; diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 2b20269ecae..b50fb2cf403 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -119,9 +119,7 @@ impl Parse for GridLine { if i.value() <= 0 { // disallow negative integers for grid spans return Err(StyleParseError::UnspecifiedError.into()) } - } else if grid_line.ident.is_some() { // integer could be omitted - grid_line.line_num = Some(Integer::new(1)); - } else { + } else if grid_line.ident.is_none() { // integer could be omitted return Err(StyleParseError::UnspecifiedError.into()) } } @@ -205,7 +203,7 @@ impl ToComputedValue for TrackBreadth { /// /// https://drafts.csswg.org/css-grid/#typedef-track-size #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)] +#[derive(Clone, Debug, HasViewportPercentage, PartialEq)] pub enum TrackSize { /// A flexible `` Breadth(TrackBreadth), @@ -213,12 +211,10 @@ pub enum TrackSize { /// and a flexible `` /// /// https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-minmax - #[css(comma, function)] Minmax(TrackBreadth, TrackBreadth), /// A `fit-content` function. /// /// https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-fit-content - #[css(function)] FitContent(L), } @@ -261,6 +257,34 @@ impl TrackSize { } } +impl ToCss for TrackSize { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + TrackSize::Breadth(ref breadth) => breadth.to_css(dest), + TrackSize::Minmax(ref min, ref max) => { + // According to gecko minmax(auto, ) is equivalent to , + // and both are serialized as . + if let TrackBreadth::Keyword(TrackKeyword::Auto) = *min { + if let TrackBreadth::Flex(_) = *max { + return max.to_css(dest); + } + } + + dest.write_str("minmax(")?; + min.to_css(dest)?; + dest.write_str(", ")?; + max.to_css(dest)?; + dest.write_str(")") + }, + TrackSize::FitContent(ref lop) => { + dest.write_str("fit-content(")?; + lop.to_css(dest)?; + dest.write_str(")") + }, + } + } +} + impl ToComputedValue for TrackSize { type ComputedValue = TrackSize; @@ -330,8 +354,13 @@ pub enum RepeatCount { impl Parse for RepeatCount { fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { - if let Ok(i) = input.try(|i| Integer::parse(context, i)) { + // Maximum number of repeat is 10000. The greater numbers should be clamped. + const MAX_LINE: i32 = 10000; + if let Ok(mut i) = input.try(|i| Integer::parse(context, i)) { if i.value() > 0 { + if i.value() > MAX_LINE { + i = Integer::new(MAX_LINE); + } Ok(RepeatCount::Number(i)) } else { Err(StyleParseError::UnspecifiedError.into()) @@ -362,7 +391,7 @@ pub struct TrackRepeat { /// If there's no ``, then it's represented by an empty vector. /// For N `` values, there will be N+1 ``, and so this vector's /// length is always one value more than that of the ``. - pub line_names: Vec>, + pub line_names: Box<[Box<[CustomIdent]>]>, /// `` values. pub track_sizes: Vec>, } @@ -423,7 +452,8 @@ impl TrackRepeat { let mut names_iter = self.line_names.iter(); for (size, names) in self.track_sizes.iter().zip(&mut names_iter) { prev_names.extend_from_slice(&names); - line_names.push(mem::replace(&mut prev_names, vec![])); + let vec = mem::replace(&mut prev_names, vec![]); + line_names.push(vec.into_boxed_slice()); track_sizes.push(size.clone()); } @@ -432,11 +462,11 @@ impl TrackRepeat { } } - line_names.push(prev_names); + line_names.push(prev_names.into_boxed_slice()); TrackRepeat { count: self.count, track_sizes: track_sizes, - line_names: line_names, + line_names: line_names.into_boxed_slice(), } } else { // if it's auto-fit/auto-fill, then it's left to the layout. @@ -517,7 +547,7 @@ pub struct TrackList { /// If there's no ``, then it's represented by an empty vector. /// For N values, there will be N+1 ``, and so this vector's /// length is always one value more than that of the ``. - pub line_names: Vec>, + pub line_names: Box<[Box<[CustomIdent]>]>, /// `` value. There can only be one `` in a TrackList. pub auto_repeat: Option>, } @@ -574,7 +604,7 @@ impl ToCss for TrackList { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct LineNameList { /// The optional `` - pub names: Vec>, + pub names: Box<[Box<[CustomIdent]>]>, /// Indicates the line name that requires `auto-fill` pub fill_idx: Option, } @@ -626,7 +656,7 @@ impl Parse for LineNameList { } Ok(LineNameList { - names: line_names, + names: line_names.into_boxed_slice(), fill_idx: fill_idx, }) } @@ -677,3 +707,13 @@ pub enum GridTemplateComponent { /// A `subgrid ?` Subgrid(LineNameList), } + +impl GridTemplateComponent { + /// Returns length of the s + pub fn track_list_len(&self) -> usize { + match *self { + GridTemplateComponent::TrackList(ref tracklist) => tracklist.values.len(), + _ => 0, + } + } +} diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index 0f1769ee1d1..81b60bdad6b 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -81,7 +81,7 @@ impl Parse for TrackSize { /// Parse the grid line names into a vector of owned strings. /// /// https://drafts.csswg.org/css-grid/#typedef-line-names -pub fn parse_line_names<'i, 't>(input: &mut Parser<'i, 't>) -> Result, ParseError<'i>> { +pub fn parse_line_names<'i, 't>(input: &mut Parser<'i, 't>) -> Result, ParseError<'i>> { input.expect_square_bracket_block()?; input.parse_nested_block(|input| { let mut values = vec![]; @@ -90,7 +90,7 @@ pub fn parse_line_names<'i, 't>(input: &mut Parser<'i, 't>) -> Result { let mut current_names; loop { - current_names = input.try(parse_line_names).unwrap_or(vec![]); + current_names = input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice()); if let Ok(track_size) = input.try(|i| TrackSize::parse(context, i)) { if !track_size.is_fixed() { if is_auto { @@ -150,7 +150,7 @@ impl TrackRepeat { // one `TrackSize`. But in current version of the spec, this is deprecated // but we are adding this for gecko parity. We should remove this when // gecko implements new spec. - names.push(input.try(parse_line_names).unwrap_or(vec![])); + names.push(input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice())); break } } else { @@ -167,7 +167,7 @@ impl TrackRepeat { let repeat = TrackRepeat { count: count, track_sizes: values, - line_names: names, + line_names: names.into_boxed_slice(), }; Ok((repeat, repeat_type)) @@ -187,6 +187,8 @@ impl Parse for TrackList { fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { // Merge the line names while parsing values. The resulting values will // all be bunch of `` and one . + // FIXME: We need to decide which way is better for repeat function in + // https://bugzilla.mozilla.org/show_bug.cgi?id=1382369. // // For example, // `[a b] 100px [c d] repeat(1, 30px [g]) [h]` will be merged as `[a b] 100px [c d] 30px [g h]` @@ -205,7 +207,7 @@ impl Parse for TrackList { let mut atleast_one_not_fixed = false; loop { - current_names.append(&mut input.try(parse_line_names).unwrap_or(vec![])); + current_names.extend_from_slice(&mut input.try(parse_line_names).unwrap_or(vec![].into_boxed_slice())); if let Ok(track_size) = input.try(|i| TrackSize::parse(context, i)) { if !track_size.is_fixed() { atleast_one_not_fixed = true; @@ -215,7 +217,8 @@ impl Parse for TrackList { } } - names.push(mem::replace(&mut current_names, vec![])); + let vec = mem::replace(&mut current_names, vec![]); + names.push(vec.into_boxed_slice()); values.push(track_size); } else if let Ok((repeat, type_)) = input.try(|i| TrackRepeat::parse_with_repeat_type(context, i)) { if list_type == TrackListType::Explicit { @@ -237,7 +240,8 @@ impl Parse for TrackList { list_type = TrackListType::Auto(values.len() as u16); auto_repeat = Some(repeat); - names.push(mem::replace(&mut current_names, vec![])); + let vec = mem::replace(&mut current_names, vec![]); + names.push(vec.into_boxed_slice()); continue }, RepeatType::Fixed => (), @@ -245,10 +249,11 @@ impl Parse for TrackList { // If the repeat count is numeric, we axpand and merge the values. let mut repeat = repeat.expand(); - let mut repeat_names_iter = repeat.line_names.drain(..); + let mut repeat_names_iter = repeat.line_names.iter(); for (size, repeat_names) in repeat.track_sizes.drain(..).zip(&mut repeat_names_iter) { current_names.extend_from_slice(&repeat_names); - names.push(mem::replace(&mut current_names, vec![])); + let vec = mem::replace(&mut current_names, vec![]); + names.push(vec.into_boxed_slice()); values.push(size); } @@ -260,7 +265,7 @@ impl Parse for TrackList { return Err(StyleParseError::UnspecifiedError.into()) } - names.push(current_names); + names.push(current_names.into_boxed_slice()); break } } @@ -268,7 +273,7 @@ impl Parse for TrackList { Ok(TrackList { list_type: list_type, values: values, - line_names: names, + line_names: names.into_boxed_slice(), auto_repeat: auto_repeat, }) } diff --git a/tests/unit/style/parsing/position.rs b/tests/unit/style/parsing/position.rs index 9029e401355..87f11b90cfb 100644 --- a/tests/unit/style/parsing/position.rs +++ b/tests/unit/style/parsing/position.rs @@ -237,7 +237,7 @@ fn test_computed_grid_template_rows_colums() { assert_computed_serialization(grid_template_rows::parse, "10px repeat(2, 1fr auto minmax(200px, 1fr))", - "10px minmax(auto, 1fr) auto minmax(200px, 1fr) minmax(auto, 1fr) auto minmax(200px, 1fr)"); + "10px 1fr auto minmax(200px, 1fr) 1fr auto minmax(200px, 1fr)"); assert_computed_serialization(grid_template_rows::parse, "subgrid [a] [] repeat(auto-fill, [])", "subgrid [a] [] repeat(auto-fill, [])");