line-height percentage value not computed accurately

RESOLVED FIXED in mozilla17

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 643238 [details]
test

Consider the attached document.  I'd expect it to print "20.5px" but instead it prints "20.4833px", one app unit off.
I get 20.5, for what it's worth.  At least on a Mac.
(Assignee)

Comment 2

5 years ago
Oh yeah, forgot to mention: this is on 32 bit Linux.  (And I think it also happens on Windows, based on try server failures of a non-reduced version of this test.)
(Assignee)

Comment 3

5 years ago
I think the issue is due to 0.411 not being exactly representable as a float.  The computation is done with:

    text->mLineHeight.SetCoordValue(
        nscoord(float(aContext->GetStyleFont()->mFont.size) *
                lineHeightValue->GetPercentValue()));

I assume different compilers are doing the floating point calculations differently, and because we're rounding down when finally converting to a nscoord and not rounding to nearest, we end up losing that one app unit.
(Assignee)

Comment 4

5 years ago
Is it reasonable to change it to use NSToCoordRound here?
(In reply to Cameron McCormack (:heycam) from comment #4)
> Is it reasonable to change it to use NSToCoordRound here?

yes
(Assignee)

Comment 6

5 years ago
Created attachment 643525 [details] [diff] [review]
patch
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #643525 - Flags: review?(dbaron)
Comment on attachment 643525 [details] [diff] [review]
patch

r=dbaron

(What adds the funny number at the end of the file?)
Attachment #643525 - Flags: review?(dbaron) → review+
(Assignee)

Comment 8

5 years ago
Ah that's git-format-patch that inserts that (and the script I have to convert the git patch to an hg patch seems to retain it).  It's the version of git that produced the patch.
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c92185fa438
https://hg.mozilla.org/mozilla-central/rev/8c92185fa438
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.