470769 - computed styles for integer-valued properties are printed in scientific (exponential) notation

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect, P4)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Computed styles for integer-valued properties are incorrectly printed in scientific (exponential) notation.  This patch fixes the problem by using doubles instead of floats so that we can accurately represent the full range of 32-bit unsigned and signed integers and thus round-trip them correctly.

We probably ought to switch to storing integers at some point, but this removes the practical problems with not doing so.

We probably also ought to ensure we never serialize to scientific notation...

(I originally put this patch in bug 373875, but that's a different bug.)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 354169 [details] [diff] [review]
patch

We could just make the float setters take a double instead of casting.  With that change, r+sr=bzbarsky

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't think any of the casts here are actually needed; I just put them in for clarity.

So I'm not sure exactly what change you wanted..

Comment 3

[Boris Zbarsky [:bzbarsky]]

I was thinking of the |void SetNumber(float aValue)| and SetPercent setters, but I'm fine with just leaving those as-is.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed:
http://hg.mozilla.org/mozilla-central/rev/441f119f1a0c
and then backed out:
http://hg.mozilla.org/mozilla-central/rev/e0740bbd7f75
http://hg.mozilla.org/mozilla-central/rev/3fdd6aaa25f1

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I relanded the test: http://hg.mozilla.org/mozilla-central/rev/82fd9752b6e1

Comment 6

[Kevin Conaway]

Is there a time table for this issue to be fixed?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

No.  Did you actually hit it in a real Web page?

Comment 8

[Kevin Conaway]

Yes.  We are using a JS library that is creating a "grid" widget with a scroll bar.  The library tries to size the scroll bar appropriately based on the number of entries in the data store.

The library sets the height of the scroll bar widget to be 40671324px which firefox then converts to 4.06713e+7px

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That's not this bug; this bug is about integer-valued properties, not length-valued properties.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In fact, it's bug 373875, which is mentioned in comment 0 of this bug.

Comment 11

[Kevin Conaway]

Sorry for the confusion, I will move my discussion there.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since I never mentioned it before:  the reason this had to be backed out was a significant number of test failures of the form:

failed | -moz-border-radius-topleft of radius4 - got "10.0000001490116%", expected "10%"

because we were converting float to double and then serializing as double.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'll write a less risky patch for this.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Store computed styles of integer-valued properties as integers in nsROCSSPrimitiveValue, so they round-trip correctly

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8398857 [details] [diff] [review]
Store computed styles of integer-valued properties as integers in nsROCSSPrimitiveValue, so they round-trip correctly

r=me.  Thank you for doing this!

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5323125a871

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b5323125a871