1766356 - ctx.font getter should return the computed fontSize value in CSS px

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[Kaiido]

Attachment: test-case.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

Set a 2D context's font property to a value with a font-size component using an unit other than CSS px (e.g cm or em).
Call the .font getter.

Actual results:

The authored value is returned. (i.e with the cm or em unit).

Expected results:

Per the specs the font setter should convert the CSS font-size component to CSS px, and at getting it should return the serialization of this value, thus with the font-size component still in CSS px.
This is problematic for instance with relative units since setting the context's font property to itself (e.g in a manual save/restore config) may result in different values being set while it shouldn't: https://jsfiddle.net/wgnvuzyp/

Comment 1

[Jamie Nicol [:jnicol]]

Thanks for the bug report Kaiido.

Lee, Jonathan, do you know how we should fix this?

Comment 2

[Jonathan Kew [:jfkthame]]

Interesting ... there's a comment in the canvas code suggesting that converting the size to pixels is an obsolete requirement. But on the other hand, the HTML canvas spec does say

the 'font-size' component converted to CSS pixels

so it seems clear we should be doing that. (Maybe the spec has changed again...)

Hiro, this seems to originate from bug 1324700; can you maybe look into the convert-to-px requirement here?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

FWIW, the comment came from the equivalent function for the old Gecko styling system; https://searchfox.org/mozilla-central/diff/b4e5e8f7289cea684a11c99340bc25e5e66f70a1/dom/canvas/CanvasRenderingContext2D.cpp#2801-2803

Anyway, honestly I don't recall how we set up the font resolving function. My (best) wild guess was resolving the given font declaration with using the parent style should fix this bug. But we do already do it. And surprisingly when I load the test case in comment 0 from my local disk, the problem doesn't happen.

If this bug is urgent, I will jump in. (Presumably just flushing pending styles before getting the parent style would solve?)

Comment 4

[Jonathan Kew [:jfkthame]]

If I'm understanding correctly, I don't think the issue here is about flushing pending styles. The problem is that Servo_SerializeFontValueForCanvas returns a serialization of the specified values (the declarations created from the .font attribute), but that means font-relative sizes like 2em are serialized as specified, not as their computed (px) value as the Canvas spec requires.

I think what needs to happen is that the serialization returned as aOutUsedFont here should use the computed value of font-size, not the specified value.

And surprisingly when I load the test case in comment 0 from my local disk, the problem doesn't happen

Weird - I don't understand this. The testcase shows the problem for me locally, whether loaded from bugzilla or my local disk; no difference.

Comment 5

[Jonathan Kew [:jfkthame]]

Fixing this should resolve three of the current firefox-only failures mentioned in bug 1646754:

/html/canvas/element/text-styles/2d.text.font.parse.size.percentage.html
/html/canvas/element/text-styles/2d.text.font.parse.size.percentage.default.html
/html/canvas/element/text-styles/2d.text.font.relative_size.html

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1766356 - Replace the specified font-size with the computed value when serializing the canvas2d font attribute. r=hiro

This requires updating a couple of our old dom/canvas mochitests that assume the size element
of the font attribute will be serialized as specified, which conflicts with the current spec
requirement.

On the other hand, this also resolves a few current WPT test failures that are specifically
testing the behavior described in the spec (e.g. a size specified as percentage gets serialized
as the computed px value).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/140b99327e33
Replace the specified font-size with the computed value when serializing the canvas2d font attribute. r=hiro

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/140b99327e33

Comment 9

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2022-04-25 Nightly build on macOS 11; Verified as fixed on Firefox 101.0b9(20220519220322) and Nightly 102.0a1(20220522190601) on Windown 10 64-bits, macOS 11 and Ubuntu 20.04.