Closed Bug 470769 Opened 11 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: dbaron, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached patch patchSplinter Review
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.)
Attachment #354169 - Flags: superreview?(bzbarsky)
Attachment #354169 - Flags: review?(bzbarsky)
Attachment #354169 - Flags: superreview?(bzbarsky)
Attachment #354169 - Flags: superreview+
Attachment #354169 - Flags: review?(bzbarsky)
Attachment #354169 - Flags: review+
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
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..
I was thinking of the |void SetNumber(float aValue)| and SetPercent setters, but I'm fine with just leaving those as-is.
Assignee: dbaron → nobody
Status: ASSIGNED → NEW
Is there a time table for this issue to be fixed?
No.  Did you actually hit it in a real Web page?
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
That's not this bug; this bug is about integer-valued properties, not length-valued properties.
In fact, it's bug 373875, which is mentioned in comment 0 of this bug.
Sorry for the confusion, I will move my discussion there.
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.
Duplicate of this bug: 679870
Duplicate of this bug: 679870
Blocks: 760121
Duplicate of this bug: 822475
Component: CSS Parsing and Computation → DOM: CSS Object Model
Priority: -- → P4
I'll write a less risky patch for this.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
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!
Attachment #8398857 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/b5323125a871
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.