font-weight should be rounded to nearest value (not floored)

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: birtles, Assigned: birtles)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
I was investigating why dom/smil/test/test_smilMappedAttrFromTo.xhtml fails with Stylo for the following test:

 new AnimTestcaseFromTo("normal", "bold",
                        { fromComp: "400", midComp: "500", toComp: "700" }),

with the following error:

  font-weight: checking value halfway through animation - got "600", expected "500"

Sure, enough, looking at the Gecko code, at 50% of the way between 400 and 700, Gecko produces 500.

The code is as follows:

> double interpolatedValue = aCoeff1 * double(aValue1.GetIntValue()) +
>                            aCoeff2 * double(aValue2.GetIntValue());

i.e. = 0.5 * 400.0 + 0.5 * 700.0
     = 200.0 + 350.0
     = 550.0

> int32_t result = floor(interpolatedValue + 0.5);

i.e. = floor(550.5)
     = 550.0

> if (aProperty == eCSSProperty_font_weight) {
>   if (result < 100) {
>     result = 100;
>   } else if (result > 900) {
>     result = 900;
>   }
>   result -= result % 100;
> } else {
>   result = RestrictValue(aProperty, result);
> }

i.e. result = 550 - 550 % 100
            = 550 - 50
            = 500

That is, we round down.

The spec, however, says:

  font weight: interpolated via discrete steps (multiples of 100). The 
  interpolation happens in real number space and is converted to an integer by
  rounding to the nearest multiple of 100, with values halfway between
  multiples of 100 rounded towards positive infinity.[1]

It looks like the code that does the rounding down was implemented in bug 528234 in Nov 2009 but the spec was updated in Mar 2012[2].

We should update Gecko to no longer floor but round and update the tests so Stylo passes too.

[1] https://drafts.csswg.org/css-transitions/#animtype-font-weight
[2] https://github.com/w3c/csswg-drafts/commit/00c62861095c9c3b911f3af0747790440a8e6467
(Assignee)

Updated

8 months ago
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

8 months ago
mozreview-review
Comment on attachment 8875995 [details]
Bug 1371196 - Round font-weight when interpolating, don't floor;

https://reviewboard.mozilla.org/r/147414/#review151634
Attachment #8875995 - Flags: review?(hikezoe) → review+

Comment 3

8 months ago
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/52c02dbc1e23
Round font-weight when interpolating, don't floor; r=hiro

Comment 4

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/52c02dbc1e23
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.