diff --git a/components/style/values/computed/length_percentage.rs b/components/style/values/computed/length_percentage.rs index 98b46454d10..4a796ba7b55 100644 --- a/components/style/values/computed/length_percentage.rs +++ b/components/style/values/computed/length_percentage.rs @@ -242,7 +242,7 @@ impl LengthPercentage { mut node: CalcNode, clamping_mode: AllowedNumericType, ) -> Self { - node.simplify_and_sort_children(); + node.simplify_and_sort(); match node { CalcNode::Leaf(l) => { diff --git a/components/style/values/generics/calc.rs b/components/style/values/generics/calc.rs index 99345b646fc..f5014a7ad08 100644 --- a/components/style/values/generics/calc.rs +++ b/components/style/values/generics/calc.rs @@ -257,11 +257,45 @@ impl CalcNode { } } - /// Simplifies and sorts the calculation. This is only needed if it's going - /// to be preserved after parsing (so, for ``). Otherwise - /// we can just evaluate it and we'll come up with a simplified value - /// anyways. - pub fn simplify_and_sort_children(&mut self) { + /// Visits all the nodes in this calculation tree recursively, starting by + /// the leaves and bubbling all the way up. + /// + /// This is useful for simplification, but can also be used for validation + /// and such. + pub fn visit_depth_first(&mut self, mut f: impl FnMut(&mut Self)) { + self.visit_depth_first_internal(&mut f); + } + + fn visit_depth_first_internal(&mut self, f: &mut impl FnMut(&mut Self)) { + match *self { + Self::Clamp { + ref mut min, + ref mut center, + ref mut max, + } => { + min.visit_depth_first_internal(f); + center.visit_depth_first_internal(f); + max.visit_depth_first_internal(f); + }, + Self::Sum(ref mut children) | Self::MinMax(ref mut children, _) => { + for child in &mut **children { + child.visit_depth_first_internal(f); + } + }, + Self::Leaf(..) => {}, + } + f(self); + } + + /// Simplifies and sorts the calculation of a given node. All the nodes + /// below it should be simplified already, this only takes care of + /// simplifying directly nested nodes. So, probably should always be used in + /// combination with `visit_depth_first()`. + /// + /// This is only needed if it's going to be preserved after parsing (so, for + /// ``). Otherwise we can just evaluate it using + /// `resolve()`, and we'll come up with a simplified value anyways. + pub fn simplify_and_sort_direct_children(&mut self) { macro_rules! replace_self_with { ($slot:expr) => {{ let dummy = Self::MinMax(Default::default(), MinMaxOp::Max); @@ -275,10 +309,6 @@ impl CalcNode { ref mut center, ref mut max, } => { - min.simplify_and_sort_children(); - center.simplify_and_sort_children(); - max.simplify_and_sort_children(); - // NOTE: clamp() is max(min, min(center, max)) let min_cmp_center = match min.partial_cmp(¢er) { Some(o) => o, @@ -323,10 +353,6 @@ impl CalcNode { return replace_self_with!(&mut **center); }, Self::MinMax(ref mut children, op) => { - for child in &mut **children { - child.simplify_and_sort_children(); - } - let winning_order = match op { MinMaxOp::Min => cmp::Ordering::Less, MinMaxOp::Max => cmp::Ordering::Greater, @@ -355,9 +381,8 @@ impl CalcNode { Self::Sum(ref mut children_slot) => { let mut sums_to_merge = SmallVec::<[_; 3]>::new(); let mut extra_kids = 0; - for (i, child) in children_slot.iter_mut().enumerate() { - child.simplify_and_sort_children(); - if let Self::Sum(ref mut children) = *child { + for (i, child) in children_slot.iter().enumerate() { + if let Self::Sum(ref children) = *child { extra_kids += children.len(); sums_to_merge.push(i); } @@ -411,6 +436,11 @@ impl CalcNode { } } + /// Simplifies and sorts the kids in the whole calculation subtree. + pub fn simplify_and_sort(&mut self) { + self.visit_depth_first(|node| node.simplify_and_sort_direct_children()) + } + fn to_css_impl(&self, dest: &mut CssWriter, is_outermost: bool) -> fmt::Result where W: Write, diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index 8732929b0cd..5ee3a11f12a 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -446,13 +446,26 @@ impl CalcNode { Ok(node) } - /// Tries to simplify this expression into a `` or ` + /// Tries to simplify this expression into a `` or `` /// value. fn into_length_or_percentage( mut self, clamping_mode: AllowedNumericType, ) -> Result { - self.simplify_and_sort_children(); + // Keep track of whether there's any invalid member of the calculation, + // so as to reject the calculation properly at parse-time. + let mut any_invalid = false; + self.visit_depth_first(|node| { + if let CalcNode::Leaf(ref l) = *node { + any_invalid |= !matches!(*l, Leaf::Percentage(..) | Leaf::Length(..)); + } + node.simplify_and_sort_direct_children(); + }); + + if any_invalid { + return Err(()); + } + Ok(CalcLengthPercentage { clamping_mode, node: self,