698652 - Minimum font size and canvas don't get along very well

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Problem: in pdf.js, we render PDFs to <canvas>.  PDFs quite commonly have most text explicitly positioned; for example, latex implements kerning by offsetting blocks of text.  This obviously leads to some serious borkenness, see https://github.com/andreasgal/pdf.js/issues/187 for more details.

I'm going to argue that minimum font size doesn't make sense for <canvas>.  Unlike HTML, there's no notion of reflow for canvas drawing, so when the UA doesn't do what the <canvas> drawer says, the result can just be totally unreadable, like what's happening in pdf.js.  There are also use cases for drawing text offscreen below the user's minimum font size, but never displaying the text.

In parallel, it would be nice to expose the minimum font size to web content, somehow.  In pdf.js, with a bit of work we can set up the zoom level so that the smallest font is the minimum font size.

I just pinged roc on IRC and he proposed the same solution, and also mentioned that we should turn off minimum font size for CSS transforms too.  I think we might want to do that for SVG as well.  Can be separate bugs I guess.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #0)
> In parallel, it would be nice to expose the minimum font size to web
> content, somehow.  In pdf.js, with a bit of work we can set up the zoom
> level so that the smallest font is the minimum font size.

The minimum font sizes are language and family-specific so that's not easy to do. At least treat it as a separate issue.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Agreed.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think this is a one-liner.  See the difference between nsStyleFont::mSize and nsStyleFont::mFont::size as documented in nsStyleStruct.h and implemented in nsRuleNode::SetFont.  To fix, change nsCanvasRenderingContext2D::SetFont to use fontStyle->mFont.mSize instead of fontStyle->mFont.size.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, should have written: ... to use fontStyle->mSize instead of fontStyle->mFont.size.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Ignore the user's minimum font size when drawing to canvas

Thanks!  I wasn't sure whether I need to continue un-zooming the mSize value, but nsStyleStruct.h suggests the only difference between the two values is due to the minimum font size, so I assume that I also need to un-zoom mSize.

dbaron, how would you recommend testing this patch?  My first inclination is to write a reftest, but that would require flipping a pref (and so would need SpecialPowers or UniversalXPConnect).

Comment 6

[Benjamin Smedberg]

See also bug 290044 for SVG.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 570942 [details] [diff] [review]
Ignore the user's minimum font size when drawing to canvas

r=dbaron.  I'd suggest changing "natural size" to "computed size".

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'd suggest using WindowSnapshot.js for tests.  Have a look at some of the existing tests that use it.  (It's basically reftest-inside-mochitest.)

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Test that minimum font size is ignored in canvas

Ugh, lost track of this patch :(.

The reftest harness grew pref() since this bug was filed, which is just what I needed.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

See also: https://github.com/mozilla/pdf.js/issues/187 (because bugzilla won't let me "See also" github URLs)

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Hi dbaron, any ETA on the review of the test?  It's pretty small, +31 lines.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 588704 [details] [diff] [review]
Test that minimum font size is ignored in canvas

change <html> to <html lang="en"> in both test and reference, and r=dbaron.

(Otherwise changing only the x-western pref shouldn't work when running tests on non-Western systems.)

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed with <html lang="en">

https://hg.mozilla.org/integration/mozilla-inbound/rev/c16a73f0f3a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8ea6b7164bc

Comment 15

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/c8ea6b7164bc
https://hg.mozilla.org/mozilla-central/rev/c16a73f0f3a1