13072 - font-size not interpreted as height of font's em-square {ll}

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Hixie (not reading bugmail)]

The test case to look at is:
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight4.html

This test page uses a font called Ahem, designed by Todd Fahrner.
Instructions for downloading the font are included on the test page itself.
The font must be installed before passing a browser through the test.

It appears that you are using the _actual_ size of the font rather than the size of
the font that you asked for, when doing calculations with the font size (e.g., for
drawing the background behind the font).

Some more test pages:
   http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/
   http://www.fas.harvard.edu/~dbaron/css/test/inlinetest
   http://www.fas.harvard.edu/~dbaron/css/test/linebox4
   http://www.fas.harvard.edu/~dbaron/css/test/emunit

Assuming fully scalable fonts are available, as they are on Windows: When
drawing a font with font-size 'x'px, you should ask the OS for a font of with
a size of 'x'px. You should assume the OS has returned a font with a size of
'x'px. The background should have a height of exactly 'x'px. Anything with a
size of 1em should have a size of exactly 'x'px.

Quick example:
   if SPAN { font-size: 50px; background: yellow; padding: 0; border: none; }
  and IMG  { height: 1em; }
 then the height of an image in the SPAN should be *exactly* 50px, and the
      background of the SPAN should be *exactly* 50px high too. Any text in
      the SPAN should have a nominal height of 50px, but this DOES NOT mean
      that the distance between the bottom of the lowest glyph to the top of
      the highest glyph will necessarily be exactly 50px, it may be a
      considerably bigger or smaller value. *This means glyphs may well spill
      out from the background*. [1]

The actual fonts may quite possibly spill into surrounding boxes, lines, text,
and so forth (assuming 'overflow' is set to 'visible', anyway).

An example of probably correct rendering of the first case listed above,
   http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/
...is Opera's rendering of the page:
   http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/opera
...(at least, assuming it is using the correct font-size).

Whatever you do, make sure you don't cause a regression in:
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight1.html
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight2.html
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight3.html

([1] To prevent glyphs from spilling out of the background, a web designer would
 use the padding properties. To prevent glyphs from flowing into surrounding
 _lines_, the web designer would use the 'line-height' property. This is not
 important for solving this bug.)

Comment 1

[Hixie (not reading bugmail)]

Kipp, I am assigning this bug to you as it is related to bug 4233, bug 5821, and
bug 6865, which you have started looking at.

Comment 2

[Hixie (not reading bugmail)]

*** Bug 4234 has been marked as a duplicate of this bug. ***

Comment 3

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

I don't think that's quite right.  You should assume the OS gives you a font the
size of what it says it gives you.  Otherwise you could have some disasters with
non-scalable fonts (i.e., many Unix boxes).

Ian spun this off of bug 4234.  I think that bug has many useful comments.  In
particular, see http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/ .

Comment 4

[Hixie (not reading bugmail)]

David: I was specifically talking of scenarios where fully scalable fonts
were  being used. Of course, when the OS actually gives you a different
font to that for which you asked, then you should adjust your actual
value of font-size to match.

Comment 5

[kipp]

Unless magic arrives, this is not likely to be fixed...so I'm dropping the
priority and milestone

Comment 6

[Masatoshi Kimura [:emk]]

Mozilla uses negative lfHeight value (excluding internal leading)
when asking for the font and uses tmHeight (including internal leading)
for the calculation. This is inconsistent.

It will solve the problem either using the positive lfHeight value
(including internal leading) or using (tmHeight - tmInternalLeading)
for the calculation.

(This is Windows only. I don't know about the other platforms.)

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: Screenshot by private build using positive lfHeight

Comment 8

[Hixie (not reading bugmail)]

Ahaha! Yes! Excellent!
Could you also attach a screenshot of the last test of:
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight4.html
...and see if that also improves?

Comment 9

[Masatoshi Kimura [:emk]]

Attachment: Screenshot for eviltest

Comment 10

[Masatoshi Kimura [:emk]]

All background boxes have exactly 100px tall :-)

Comment 11

[Hixie (not reading bugmail)]

VYV03354: Looks to me like that is the correct fix. Have you seen any problems
with this change and browsing the top 100 or any of the test cases mentioned at
the top of this bug report?

If not, I suggest you attach a diff...

Comment 12

[Michael Lowe]

CCing some experienced font people for advise.  Hope they don't mind.

Comment 13

[Erik van der Poel]

VYV03354, what is your name? Thank you for the info and for trying a fix!

I checked the documentation of lfHeight and tmHeight, and you appear to be
correct. Also, it is correct to use the positive lfHeight value, since this
includes the "internal leading", which includes any accents that might appear
above letters.

VYV03354, do you have check-in privileges? If not, please attach, include or
send your diff. Thank you!

Comment 14

[Masatoshi Kimura [:emk]]

Attachment: Patch using the positive lfHeight

Comment 15

[Masatoshi Kimura [:emk]]

To enable this patch, set "GECKO_USE_POSITIVE_LFHEIGHT=1".
(Please call me "emk".)

Comment 16

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

Marking All/All since this problem also occurs on Linux.

Comment 17

[Erik van der Poel]

I started a discussion on the [e-mail] mailing list:

  http://lists.w3.org/Archives/Public/www-style/1999Nov/0104.html
  http://lists.w3.org/Archives/Public/www-style/1999Nov/0105.html
  http://lists.w3.org/Archives/Public/www-style/1999Nov/0106.html

My conclusion is that the negative lfHeight is correct. However, the claim is
that Mozilla is using tmHeight elsewhere, so this is inconsistent (since it
includes the internal leading). So this still needs to be investigated.

Also, on X we are using (max ascent + max descent) but this should be (font
ascent + font descent).

Finally, we should check the Mac version to see what it's doing.

Comment 18

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

VYV03354 - do you mind if I use the screenshot you provided as a reference
rendering on http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/ ?

Comment 19

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

For future reference, documentation for the various Windows font API things
mentioned above is at (as Erik mentioned on www-style):

http://msdn.microsoft.com/library/psdk/gdi/fontext_1wmq.htm  (LOGFONT)
http://msdn.microsoft.com/library/psdk/gdi/fontext_2zqq.htm  (NEWTEXTMETRIC)

Comment 20

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

For future reference, some unix documentation is at:

http://frcatel.utc.sk/xlib/graphics/font-metrics/

in addition to the man page for "XLoadFont".

Comment 21

[Masatoshi Kimura [:emk]]

>VYV03354 - do you mind if I use the screenshot you provided as a reference
>rendering on http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/ ?

No, not at all.

Comment 22

[Erik van der Poel]

On Windows, nsIFontMetrics' GetHeight, GetMaxAscent and GetMaxDescent return the
max height, ascent and descent values. On Unix, we were using ascent and descent
rather than max_bounds.ascent and .descent, so I checked in a fix to at least
make the Unix version consistent with the Windows version for now. I also
changed the Unix version to use ascent and descent instead of their max values
when *specifying* the font size (as opposed to querying it). This is also more
consistent with the Windows version.

The font-size debate is still unresolved on [e-mail]. I will leave the
Windows and Unix versions as they are now until we have some resolution.

Comment 23

[Hixie (not reading bugmail)]

Migrating from {css1} to css1 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...

Comment 24

[Erik van der Poel]

Moving all M16s to M17. Please make comments if you disagree.

Comment 25

[Erik van der Poel]

We need to resolve more of the font size problems for Beta 1.

Comment 26

[leger]

Putting on PDT+ radar for beta1.

Comment 27

[Erik van der Poel]

Attachment: Vertical ruler image for test.

Comment 28

[Erik van der Poel]

Attachment: New test case.

Comment 29

[Erik van der Poel]

QA folks: I'd like somebody to run my latest test case (see attachments) on the
Mac. Please make sure there are 10.5 lines of text next to the ruler. Let me
know the results! Thanks.

Comment 30

[Erik van der Poel]

I have fixed the font size bug on Unix, and I have opened bug 27164 to deal with
nsIFontMetrics::GetHeight, which currently returns the bounding box height, but
should probably be changed to GetEmHeight, to return the em height of the
English font (used only in specific contexts e.g. margin: 1.4em).

I am closing this bug. It is rather long and unwieldy. If there are other
specific bugs, let's open new ones for them. For example, if the Mac still
doesn't produce 10.5 lines in my test case above, please open a new bug.

Comment 31

[Christine Hoffman]

Erik: I'm qa on this bug. I'm not sure how I should be verifying this bug. I am 
testing on Mac8.5 with the 02-15-16 build. I count 11 + lines of text next to 
the ruler. Does that mean there is still a problem with this bug? What is the 
remaining problem with this bug after the font size bug on Unix and the 
nsIFontMetrics::GetHeight problem

Comment 32

[Erik van der Poel]

Hi Chris (QA), Ian and I have created several new bugs so that this one can be
closed. The only one that we haven't created is the Mac bug (your 11+ lines
bug), so could you open that separately, and assign it to pierre please? Ian,
if you agree that this bug can be closed, please close it. Thanks!

Comment 33

[Hixie (not reading bugmail)]

erik: to verify this bug, would one not need to make sure your new test case --
   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=5045
-- displays correctly in Unix?

Comment 34

[Erik van der Poel]

I created bug 27551 for the X font scaling issue. (UnixMoz is using bitmap fonts
that are too large or too small in some cases, so we should try to use scalable
fonts if they are of high quality.)

Comment 35

[Hixie (not reading bugmail)]

VERIFIED that this bug has been splintered into billions of other bugs and that
every known font-size issue is now another bug and so this bug can be closed.

;-)

Comment 36

[rbs]

Has there been any regression with this bug? Visiting with a tip of trunk
http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/
I don't get the reference attachment 2758 [details].

Comment 37

[rbs]

Attachment: current rendition - does this pass the test too?

Comment 38

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

That was based on an idealized notion of the metrics that fonts should provide.
 Unfortunately, they don't work that way, and can only be forced to work that
way on Windows (maybe Mac too -- I'm not sure).