Bug 216601 – getComputedStyle rounds lineHeight to nearest pixel

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Neil Ashford]

See example: https://jsfiddle.net/kyrfgot6/1/

When calling `getComputedStyle(element).lineHeight` the output is rounded to the nearest pixel value. Other browsers give a precise value, and I need it to be precise so that I can calculate the original (unitless) line height specified in the CSS.

[Radar WebKit Bug Importer]

<rdar://problem/69022887>

[Tyler Wilcock]

Created attachment 414781 [details]
Patch

[Tyler Wilcock]

Created attachment 414782 [details]
Patch

[Tyler Wilcock]

Created attachment 414914 [details]
Patch

[Tyler Wilcock]

Thanks for the review, Simon!  Just rebased the patch, will mark cq? when the tests are green again.

[Antti Koivisto]

Comment on attachment 414914 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414914&action=review

> LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-computed-expected.txt:10
> -FAIL Property font value 'normal normal xx-large/1.2 cursive' assert_equals: expected "32px / 38px cursive" but got "normal normal normal normal 32px/38px cursive"
> +FAIL Property font value 'normal normal xx-large/1.2 cursive' assert_equals: expected "32px / 38.400001525878906px cursive" but got "normal normal normal normal 32px/38.400001525878906px cursive"

We appear to be failing these WPT tests harder than before?

[Tyler Wilcock]

(In reply to Antti Koivisto from comment #6)
> Comment on attachment 414914 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414914&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-computed-expected.txt:10
> > -FAIL Property font value 'normal normal xx-large/1.2 cursive' assert_equals: expected "32px / 38px cursive" but got "normal normal normal normal 32px/38px cursive"
> > +FAIL Property font value 'normal normal xx-large/1.2 cursive' assert_equals: expected "32px / 38.400001525878906px cursive" but got "normal normal normal normal 32px/38.400001525878906px cursive"
> 
> We appear to be failing these WPT tests harder than before?

How so?  With this patch, both the expectations and results for line-height values have been updated to be fractional.  Both Gecko and Chromium also serialize these fractionally for this test.  Maybe I'm missing your point, though -- can you elaborate?

wpt.fyi: https://wpt.fyi/results/css/css-fonts/parsing/font-computed.html?label=master&label=experimental&aligned&q=css%2Fcss-fonts

Test source: https://github.com/web-platform-tests/wpt/blob/master/css/css-fonts/parsing/font-computed.html

[EWS]

Committed r270248: <https://trac.webkit.org/changeset/270248>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414914 [details].

[Antti Koivisto]

> How so?  With this patch, both the expectations and results for line-height
> values have been updated to be fractional.  Both Gecko and Chromium also
> serialize these fractionally for this test.  Maybe I'm missing your point,
> though -- can you elaborate?

Based on the output the expected value in the test is 32px/38px which is what we were producing before this patch. Are you saying that both Chrome and Gecko now fail the same way as we do? Or that our version of the test is out of date? If the latter, you should update it.

[Tyler Wilcock]

(In reply to Antti Koivisto from comment #9)

> Based on the output the expected value in the test is 32px/38px which is what we were producing before this patch.
The expected value in this test is not hardcoded, it is generated by a "reference" style vs. the "under-test" style.  So it's not like this:

assert_equals('...32px / 38px...',  'actual value')

and is instead like this (pseudocode):

assert_equals(getComputedStyle(reference)['font'], getComputedStyle(actual)['font])

So by changing line-height to serialize as a double instead of an int, we change both the reference-expectation and the actual value to a fractional value.

> Are you saying that both Chrome and Gecko now fail the same way as we do?
Using the case quoted above as an example:
WebKit before this patch: 32px / 38px
WebKit after this patch: 32px / 38.400001525878906px
Gecko: 32px / 38.4px
Chromium: 32px / 38.4px

Point being, we now represent line-height fractionally as others do.  Chromium passes this case, and Gecko fails for unrelated reasons.

> Or that our version of the test is out of date?
Our version of the test is indeed the latest, so no concerns there.

---

Does this clear it up, or am I still missing your point?

[Antti Koivisto]

> Does this clear it up, or am I still missing your point?

Yep, that makes sense. Thanks for explaining.