diff --git a/servo/components/style/values/generics/calc.rs b/servo/components/style/values/generics/calc.rs index b0f15e5956201..9a3d7d793aac1 100644 --- a/servo/components/style/values/generics/calc.rs +++ b/servo/components/style/values/generics/calc.rs @@ -571,26 +571,29 @@ impl CalcNode { /// Tries to merge one node into another using the product, that is, perform `x` * `y`. pub fn try_product_in_place(&mut self, other: &mut Self) -> bool { - if let CalcNode::Leaf(left) = self { - if let CalcNode::Leaf(right) = other { - return left.try_product_in_place(right); + if let Ok(resolved) = other.resolve() { + if let Some(number) = resolved.as_number() { + if number == 1.0 { + return true; + } + + if self.is_product_distributive() { + self.map(|v| v * number); + return true; + } } } - if let CalcNode::Leaf(left) = self { - if let Some(left) = left.as_number() { - if other.is_product_distributive() || left == 1.0 { - other.map(|v| v * left); + if let Ok(resolved) = self.resolve() { + if let Some(number) = resolved.as_number() { + if number == 1.0 { std::mem::swap(self, other); return true; } - } - } - if let CalcNode::Leaf(right) = other { - if let Some(right) = right.as_number() { - if self.is_product_distributive() || right == 1.0 { - self.map(|v| v * right); + if other.is_product_distributive() { + other.map(|v| v * number); + std::mem::swap(self, other); return true; } } diff --git a/servo/components/style/values/specified/calc.rs b/servo/components/style/values/specified/calc.rs index 4da1d1e856c6c..91682d889ca67 100644 --- a/servo/components/style/values/specified/calc.rs +++ b/servo/components/style/values/specified/calc.rs @@ -780,46 +780,57 @@ impl CalcNode { Ok(&Token::Delim('*')) => { let mut rhs = Self::parse_one(context, input, allowed_units)?; - let last = product.last_mut().unwrap(); + // We can unwrap here, becuase we start the function by adding a node to + // the list. + if !product.last_mut().unwrap().try_product_in_place(&mut rhs) { + product.push(rhs); + } + }, + Ok(&Token::Delim('/')) => { + let rhs = Self::parse_one(context, input, allowed_units)?; - if let Ok(value) = rhs.to_number() { - if last.is_product_distributive() || value == 1.0 { - last.map(|v| v * value); - continue; - } + enum InPlaceDivisionResult { + /// The right was merged into the left. + Merged, + /// The right is not a number or could not be resolved, so the left is + /// unchanged. + Unchanged, + /// The right was resolved, but was not a number, so the calculation is + /// invalid. + Invalid, } - if let Ok(value) = last.to_number() { - if value == 1.0 { - std::mem::swap(last, &mut rhs); - continue; - } - if rhs.is_product_distributive() { - rhs.map(|v| v * value); - std::mem::swap(last, &mut rhs); - continue; + fn try_division_in_place( + left: &mut CalcNode, + right: &CalcNode, + ) -> InPlaceDivisionResult { + if let Ok(resolved) = right.resolve() { + if let Some(number) = resolved.as_number() { + if number != 1.0 && left.is_product_distributive() { + left.map(|l| l / number); + return InPlaceDivisionResult::Merged; + } + } else { + return InPlaceDivisionResult::Invalid; + } } + InPlaceDivisionResult::Unchanged } - product.push(rhs); - }, - Ok(&Token::Delim('/')) => { - let rhs = Self::parse_one(context, input, allowed_units)?; - - if let (CalcNode::Leaf(left), Ok(right)) = - (product.last_mut().unwrap(), rhs.to_number()) - { - left.map(|l| l / right); - } else { - // Right hand side is not resolvable to a number right now, but it must - // still resolve to a number eventually, so check it. - if !rhs.unit().map_or(false, |n| n.is_empty()) { + // The right hand side of a division *must* be a number, so if we can + // already resolve it, then merge it with the last node on the product list. + // We can unwrap here, becuase we start the function by adding a node to + // the list. + match try_division_in_place(product.last_mut().unwrap(), &rhs) { + InPlaceDivisionResult::Merged => {}, + InPlaceDivisionResult::Unchanged => { + product.push(Self::Invert(Box::new(rhs))) + }, + InPlaceDivisionResult::Invalid => { return Err( input.new_custom_error(StyleParseErrorKind::UnspecifiedError) - ); - } - - product.push(Self::Invert(Box::new(rhs))); + ) + }, } }, _ => {