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

RESOLVED FIXED in mozilla12

Status

()

Core
Canvas: 2D
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
(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.
Agreed.
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.
Er, should have written: ... to use fontStyle->mSize instead of fontStyle->mFont.size.
Created attachment 570942 [details] [diff] [review]
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).
Assignee: nobody → jones.chris.g
Attachment #570942 - Flags: review?(dbaron)
See also bug 290044 for SVG.
Review ping.
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".
Attachment #570942 - Flags: review?(dbaron) → review+
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.)
Created attachment 588704 [details] [diff] [review]
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.
Attachment #588704 - Flags: review?(dbaron)
See also: https://github.com/mozilla/pdf.js/issues/187 (because bugzilla won't let me "See also" github URLs)
Hi dbaron, any ETA on the review of the test?  It's pretty small, +31 lines.
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.)
Attachment #588704 - Flags: review?(dbaron) → review+
Pushed with <html lang="en">

https://hg.mozilla.org/integration/mozilla-inbound/rev/c16a73f0f3a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8ea6b7164bc
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/c8ea6b7164bc
https://hg.mozilla.org/mozilla-central/rev/c16a73f0f3a1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla12
Blocks: 722569
You need to log in before you can comment on or make changes to this bug.