Closed Bug 1778908 Opened 2 years ago Closed 2 years ago

Implement the fontKerning attribute for Canvas2D text

Categories

(Core :: Graphics: Canvas2D, defect)

defect

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox104 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

No description provided.

Note that Chrome's implementation of fontKerning treats the attribute values as case-insensitive, and the WPT tests they landed expect this. I believe that's wrong, and have filed https://bugs.chromium.org/p/chromium/issues/detail?id=1343333 about it.

Hi there, assigning a severity as part of triage, since this is a defect I've assigned S4 because I think this is a cosmetic issue if I am understanding the definitions correctly, but I know that kerning can lead to not-so-subtle layout changes, do we have an example of this defect in the wild?

Severity: -- → S4

Per spec, the value of fontKerning should be an enum, not a string, but currently we handle
all the keyword-valued canvas attributes in this way. We may want to convert them to enums
(which will mean that unrecognized values throw an error instead of being ignored), but I
think that should be done for all the attributes together as a separate bug. For now, using
a string here provides consistency with the rest of the canvas APIs.

Note that Blink's current implementation and the existing WPT tests have some problems:
they treat the values of fontKerning as case-insensitive, which is wrong. I have filed
https://bugs.chromium.org/p/chromium/issues/detail?id=1343333 about this.

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28bb76d444f5
Implement the fontKerning attribute for Canvas2d text. r=gfx-reviewers,aosmond,emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34838 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch
Upstream PR merged by moz-wptsync-bot

FF104 docs for this feature can be tracked in https://github.com/mdn/content/issues/18772

I think we need to add the attribute to CanvasRenderingContext2D, ImageBitmapRenderingContext, WebGLRenderingContext, WebGL2RenderingContext for docs and browser compatibility data. Is that correct?

We should also add it to GPUCanvasContext but since this is the WebGPU API, which I don't think Firefox implements, presumably it is not present there? Is there anywhere else I have missed?

In addition, it sounds like fontKerning is implemented as strings by FF and Chrome rather than enums as per spec. As MDN documents the spec but everyone does the "wrong thing" here I will add the exceptions for this AND add a note that they may not be thrown - see compat table. I'll also note the behaviour for chrome and FF in the browser compatibility table.

Last of all, for browser compat I'll note that Chrome accepts case insensitive values as per https://bugs.chromium.org/p/chromium/issues/detail?id=1343333. Just FMI, it looks like you have told them there to fix the case-insensitivity but NOT that they need to throw rather than ignore invalid values. Do we need to note that in their bug?

Thanks!

Flags: needinfo?(jfkthame)

As far as I'm aware, this only applies to [Offscreen]CanvasRenderingContext2D, not the other contexts, as it is part of the CanvasTextDrawingStyles APIs, which are used only by the 2D rendering contexts. So it's not relevant to (or exposed on) image or webgl contexts.

Regarding the string vs enum spec-compliance issue and the associated error-handling behavior, that's already noted in the Chrome bug (see https://bugs.chromium.org/p/chromium/issues/detail?id=1343333#c7 and following comments).

Flags: needinfo?(jfkthame)

Thanks Jonathan, I don't think that is what the spec says:

The fontKerning attribute is defined in the mixin CanvasTextDrawingStyles, which is directly inherited by CanvasRenderingContext2D and also by OffscreenCanvasRenderingContext2D which is typedefed to ImageBitmapRenderingContext, WebGLRenderingContext, WebGL2RenderingContext or GPUCanvasContext (this is in the WebGPU API, which is not documented at all).

Doesn't that mean that it "should" be in all those interfaces? Or do you mean that it might be, but we only implement it in CanvasRenderingContext2D?

Flags: needinfo?(jfkthame)

OffscreenCanvasRenderingContext2D is not typedefed to anything, it's its own interface, what am I missing? Your own link points to the right bit in the spec: https://html.spec.whatwg.org/multipage/canvas.html#offscreencanvasrenderingcontext2d

The typedef is https://html.spec.whatwg.org/multipage/canvas.html#renderingcontext

The CanvasTextDrawingStyles mixin is used by CanvasRenderingContext2D and OffscreenCanvasRenderingContext2D only. These are entirely distinct from the other (image/webgl/gpu) contexts, not inherited by them or anything like that.

Flags: needinfo?(jfkthame)

Arrrg. Thanks. I was misinterpreting https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface:

typedef (OffscreenCanvasRenderingContext2D or ImageBitmapRenderingContext or WebGLRenderingContext or WebGL2RenderingContext or GPUCanvasContext) OffscreenRenderingContext;

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: