1834164 - Serialize NaN and infinity numbers

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Oliver Medhurst [:canadahonk]]

<number> values like NaN and infinity (and -infinity) should be serialized properly as per spec, see WPT tests for examples. opacity: number and transform: scale(number) can be used as number examples. There are no WPT tests just for this, they should be added (something like calc-infinity-nan-serialize-number.html, see similar existing tests).

Some current existing WPT tests show this:

• https://wpt.fyi/results/css/css-values/exp-log-serialize.html (3rd and 4th last)
• https://wpt.fyi/results/css/css-values/sin-cos-tan-serialize.html (the last FF failing set)
• Potentially more random/uncaught subtests, should retest all css-values WPTs

Comment 1

[Oliver Medhurst [:canadahonk]]

Changed scope of this bug to just numbers as our existing NaN/inf util funcs are for floats and integers are more complex and require more investigation into expected/spec behavior with NaN/inf.

Comment 2

[Oliver Medhurst [:canadahonk]]

Attachment: Bug 1834164 - Serialize NaN and infinity numbers

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.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff7a5040b749
Serialize NaN and infinity numbers r=emilio

Comment 4

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40139 for changes under testing/web-platform/tests

Comment 5

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages

Backout link

Push with failures

Failure log

Comment 6

[Oliver Medhurst [:canadahonk]]

Apologies, fixed in patch update.

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a8d978dd79c
Serialize NaN and infinity numbers r=emilio

Comment 9

[Norisz Fay [:noriszfay]]

Backed out for causing multiple failures

Backout link

Push with failures -R5
Push with failure - mochitest
Push with failure - C

Failure log R5 // Failure log C // Failure log mochitest

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 11

[Oliver Medhurst [:canadahonk]]

Changed patch to WIP as this broke more (reftests) than first seemed due to some things not expecting NaN/inf values.

Comment 12

[Daniel Holbert [:dholbert] (PTO until 2/24)]

(In reply to CanadaHonk [:canadahonk] from comment #11)

Changed patch to WIP as this broke more (reftests) than first seemed due to some things not expecting NaN/inf values.

Looks like it changed back from WIP. :) Is this good to re-land?

Comment 13

[Oliver Medhurst [:canadahonk]]

Sorry for delay, was refraining a bit due to the "troubles" from this. Landing again :)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1016ade4b638
Serialize NaN and infinity numbers r=emilio

Comment 15

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/1016ade4b638

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:canadahonk, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Comment 18

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Not worth uplifting; this can ride the trains as normal.

Comment 19

[Daniel Holbert [:dholbert] (PTO until 2/24)]

(In reply to CanadaHonk [:canadahonk] from comment #0)

Some current existing WPT tests show this:

• https://wpt.fyi/results/css/css-values/exp-log-serialize.html (3rd and 4th last)

Looks like these are now fixed. http://wpt.live/css/css-values/exp-log-serialize.html now only shows 2 failures, the very last 2, which fail in the same way among all browsers I think.

• https://wpt.fyi/results/css/css-values/sin-cos-tan-serialize.html (the last FF failing set)

I don't think these ones ended up being fixed here. We pass 52/68 test in this WPT (failing 16), both before and after this patch. It looks like bug 1834361 covers this part --> adding that as a dependency, since it seems it'll build on top of the work here.

Comment 20

[Daniel Holbert [:dholbert] (PTO until 2/24)]

(Thanks for the fix, BTW! :) )