Skip to content

Commit

Permalink
Bug 1834164 - Serialize NaN and infinity numbers r=emilio
Browse files Browse the repository at this point in the history
Added NaN/inf serialization of <number> and changed calc() code to not
remove NaN/infinity in code using it.

This change is unfortunately imperfect as some things using <number>
still refuse to serialize NaN/infinity for some reason (scale()?), but
this bug/patch is just for <number> so leaving that out of scope for this.

Also added new WPT test file for number NaN/inf serialization based
on existing serialization tests (all pass already!).

5 other WPT subtests now newly pass.

Differential Revision: https://phabricator.services.mozilla.com/D178587
  • Loading branch information
CanadaHonk committed May 22, 2023
1 parent 71f8946 commit c0c95c1
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 37 deletions.
18 changes: 15 additions & 3 deletions servo/components/style/values/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ fn nan_inf_enabled() -> bool {
static_prefs::pref!("layout.css.nan-inf.enabled")
}

/// Serialize a number with calc, and NaN/infinity handling (if enabled)
pub fn serialize_number<W>(v: f32, was_calc: bool, dest: &mut CssWriter<W>) -> fmt::Result
where
W: Write,
{
serialize_specified_dimension(v, "", was_calc, dest)
}

/// Serialize a specified dimension with unit, calc, and NaN/infinity handling (if enabled)
pub fn serialize_specified_dimension<W>(v: f32, unit: &str, was_calc: bool, dest: &mut CssWriter<W>) -> fmt::Result
where
Expand All @@ -112,11 +120,15 @@ where
// requires an expression like calc(infinity * 1px)."

if v.is_nan() {
dest.write_str("NaN * 1")?;
dest.write_str("NaN")?;
} else if v == f32::INFINITY {
dest.write_str("infinity * 1")?;
dest.write_str("infinity")?;
} else if v == f32::NEG_INFINITY {
dest.write_str("-infinity * 1")?;
dest.write_str("-infinity")?;
}

if !unit.is_empty() {
dest.write_str(" * 1")?;
}
} else {
v.to_css(dest)?;
Expand Down
17 changes: 11 additions & 6 deletions servo/components/style/values/specified/calc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::values::generics::calc::{MinMaxOp, ModRemOp, RoundingStrategy, SortKe
use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength};
use crate::values::specified::length::{ContainerRelativeLength, ViewportPercentageLength};
use crate::values::specified::{self, Angle, Resolution, Time};
use crate::values::{CSSFloat, CSSInteger};
use crate::values::{CSSFloat, CSSInteger, serialize_percentage, serialize_number};
use cssparser::{AngleOrNumber, CowRcStr, NumberOrPercentage, Parser, Token};
use smallvec::SmallVec;
use std::cmp;
Expand Down Expand Up @@ -104,9 +104,9 @@ impl ToCss for Leaf {
{
match *self {
Self::Length(ref l) => l.to_css(dest),
Self::Number(ref n) => n.to_css(dest),
Self::Number(n) => serialize_number(n, /* was_calc = */ false, dest),
Self::Resolution(ref r) => r.to_css(dest),
Self::Percentage(p) => crate::values::serialize_percentage(p, dest),
Self::Percentage(p) => serialize_percentage(p, dest),
Self::Angle(ref a) => a.to_css(dest),
Self::Time(ref t) => t.to_css(dest),
}
Expand Down Expand Up @@ -860,10 +860,16 @@ impl CalcNode {

/// Tries to simplify this expression into a `<number>` value.
fn to_number(&self) -> Result<CSSFloat, ()> {
self.resolve(|leaf| match *leaf {
let number = self.resolve(|leaf| match *leaf {
Leaf::Number(n) => Ok(n),
_ => Err(()),
})
})?;
let result = if nan_inf_enabled() {
number
} else {
crate::values::normalize(number)
};
Ok(result)
}

/// Tries to simplify this expression into a `<percentage>` value.
Expand Down Expand Up @@ -965,7 +971,6 @@ impl CalcNode {
) -> Result<CSSFloat, ParseError<'i>> {
Self::parse(context, input, function, CalcUnits::empty())?
.to_number()
.map(crate::values::normalize)
.map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
}

Expand Down
19 changes: 6 additions & 13 deletions servo/components/style/values/specified/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use super::generics::{self, GreaterThanOrEqualToOne, NonNegative};
use super::{CSSFloat, CSSInteger};
use crate::context::QuirksMode;
use crate::parser::{Parse, ParserContext};
use crate::values::serialize_atom_identifier;
use crate::values::{serialize_atom_identifier, serialize_number};
use crate::values::specified::calc::CalcNode;
use crate::{Atom, Namespace, One, Prefix, Zero};
use cssparser::{Parser, Token};
Expand Down Expand Up @@ -196,15 +196,15 @@ fn parse_number_with_clamping_mode<'i, 't>(
match *input.next()? {
Token::Number { value, .. } if clamping_mode.is_ok(context.parsing_mode, value) => {
Ok(Number {
value: value.min(f32::MAX).max(f32::MIN),
value,
calc_clamping_mode: None,
})
},
Token::Function(ref name) => {
let function = CalcNode::math_function(context, name, location)?;
let result = CalcNode::parse_number(context, input, function)?;
let value = CalcNode::parse_number(context, input, function)?;
Ok(Number {
value: result.min(f32::MAX).max(f32::MIN),
value,
calc_clamping_mode: Some(clamping_mode),
})
},
Expand Down Expand Up @@ -299,7 +299,7 @@ impl ToComputedValue for Number {

#[inline]
fn to_computed_value(&self, _: &Context) -> CSSFloat {
self.get()
crate::values::normalize(self.get())
}

#[inline]
Expand All @@ -316,14 +316,7 @@ impl ToCss for Number {
where
W: Write,
{
if self.calc_clamping_mode.is_some() {
dest.write_str("calc(")?;
}
self.value.to_css(dest)?;
if self.calc_clamping_mode.is_some() {
dest.write_char(')')?;
}
Ok(())
serialize_number(self.value, self.calc_clamping_mode.is_some(), dest)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,3 @@

['scale(calc(log(0)))' as a computed value should serialize as 'matrix(-infinity, 0, 0, -infinity, 0, 0)'.]
expected: FAIL

['calc(log(0))' as a specified value should serialize as 'calc(-infinity)'.]
expected: FAIL

['scale(calc(log(0)))' as a specified value should serialize as 'scale(calc(-infinity))'.]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,9 @@
['scale(calc(rem(1,0)))' as a computed value should serialize as 'matrix(NaN, 0, 0, NaN, 0, 0)'.]
expected: FAIL

['calc(rem(1,0))' as a specified value should serialize as 'calc(NaN)'.]
expected: FAIL

['calc(rem(1,0))' as a computed value should serialize as 'NaN'.]
expected: FAIL

['calc(mod(1,0))' as a specified value should serialize as 'calc(NaN)'.]
expected: FAIL

['calc(round(1,0))' as a specified value should serialize as 'calc(NaN)'.]
expected: FAIL

['scale(calc(mod(1,0)))' as a specified value should serialize as 'scale(calc(NaN))'.]
expected: FAIL

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!DOCTYPE HTML>
<title>Infinity and NaN: calc() serialization for number values.</title>
<link rel="help" href="https://drafts.csswg.org/css-values/#calc-type-checking">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../support/serialize-testcommon.js"></script>
<div id="target"></div>
<div id="log"></div>
<script>
function test_serialization(t,s, {prop="opacity"}={}) {
test_specified_serialization(prop, t, s)
}
//TEST CASE | EXPECTED
var test_map = {
"NaN" :"calc(NaN)",
"infinity" :"calc(infinity)",
"-infinity" :"calc(-infinity)",
"1 * NaN" :"calc(NaN)",
"1 * nan" :"calc(NaN)",
"1 * infinity / infinity" :"calc(NaN)",
"1 * 0 * infinity" :"calc(NaN)",
"1 * (infinity + -infinity)" :"calc(NaN)",
"1 * (-infinity + infinity)" :"calc(NaN)",
"1 * (infinity - infinity)" :"calc(NaN)",
"1 * infinity" :"calc(infinity)",
"1 * -infinity" :"calc(-infinity)",
"1 * iNFinIty" :"calc(infinity)",
"1 * (infinity + infinity)" :"calc(infinity)",
"1 * (-infinity + -infinity)" :"calc(-infinity)",
"1 * 1/infinity" :"calc(0)",
"1 * infinity * infinity" :"calc(infinity)",
"1 * -infinity * -infinity" :"calc(infinity)",
"1 * max(INFinity*3, 0)" :"calc(infinity)",
"1 * min(inFInity*4, 0)" :"calc(0)",
"1 * max(nAn*2, 0)" :"calc(NaN)",
"1 * min(nan*3, 0)" :"calc(NaN)",
"1 * clamp(-INFINITY*20, 0, infiniTY*10)" :"calc(0)",

"1 * max(NaN, min(0,10))" :"calc(NaN)",
"1 * clamp(NaN, 0, 10)" :"calc(NaN)",

"1 * max(0, min(10, NaN))" :"calc(NaN)",
"1 * clamp(0, 10, NaN)" :"calc(NaN)",

"1 * max(0, min(NaN, 10))" :"calc(NaN)",
"1 * clamp(0, NaN, 10)" :"calc(NaN)",

"1 * clamp(-Infinity, 0, infinity)" :"calc(0)",
"1 * clamp(-inFinity, infinity, 10)" :"calc(10)",
};

for (var exp in test_map) {
test_serialization("calc("+exp+")", test_map[exp]);
}
</script>

0 comments on commit c0c95c1

Please sign in to comment.