Closed Bug 27164 Opened 25 years ago Closed 23 years ago

line-height should be multiplied by em square height {ll} [INLINE]

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED WORKSFORME
Milestone:
mozilla1.0

People

(Reporter: erik, Assigned: shanjian)

References

(
URL
)

Details

(4 keywords, Whiteboard: [Hixie-P1] (most important of two remaining inline box model bug))

Attachments

(1 file)

test case (Times New Roman, with WinIE and WinOpera measurements)
1.44 KB, text/html
Details 
This bug has been opened to remind us to clean up the nsIFontMetrics API, which
contains GetHeight and so on. We may want to change this to GetEmHeight, and
include a comment that this is for the English font only. More ideas to come
later, since this is not an M14 priority for now.
Status: NEW → ASSIGNED
Target Milestone: M15
Blocks: 27874
Blocks: 27873
Summary: update nsIFontMetrics API → update nsIFontMetrics API {ll}
Target M16, after Beta 1 branch, in conjunction with bug 20394.
Target Milestone: M15 → M16
Added a few people to the Cc field. I'd like to make a proposal for the line
height problem, and if you agree, I'll proceed. First, an explanation:

Here are the current height-related APIs:

  GetHeight, GetLeading, GetMaxAscent, GetMaxDescent

Basically, the problem is that GetHeight returns the max height, and we are
multiplying this value by the CSS line-height property to compute the line
spacing. There have been a number of long discussions about this on the
www-style@w3.org mailing list, and it seems that many people believe that the
line spacing should be computed by multiplying line-height by the em square
height, which is different from the max height. IE and Opera also use the em
square height to compute the line spacing.

For line-height "normal", Mozilla currently uses the value 1.0 to multiply by
the return value of GetHeight. For the default font (Times New Roman) at the
default size (12pt == 16px @ 96ppi), GetHeight returns 19, and this is the same
as the line spacing used by WinNav4 and WinIE in CSS-less HTML.

However, this is not the line spacing recommended by Times New Roman itself. It
recommends 20 (when the size is 16). I'd like to propose that we try the
recommended line spacing, and see how that turns out. If we have too many
compatibility problems, we can revert to using the max height as the line
spacing, possibly only on Windows (where we seem to have the strictest
compatibility requirements).

Whether we go with the recommended line spacing or not, I'd like to propose a
number of new methods, so that the new set of height-related APIs becomes:

  GetMaxHeight, GetMaxAscent, GetMaxDescent
  GetEmHeight, GetEmAscent, GetEmDescent
  GetLeading

For line-height "normal", we should use GetEmHeight + GetLeading. Alternatively,
we could have a single API that returns that sum, called GetNormalLineHeight.
Initially, I'd like to add the new APIs. Then move all the callers to these new
methods, and finally remove the old API (GetHeight).

Please let me know whether you agree with this plan. If there is too much
disagreement, we can either postpone it to Netscape 7 or cancel it all together.
*** Bug 27874 has been marked as a duplicate of this bug. ***
Summary: update nsIFontMetrics API {ll} → line-height should be multiplied by em square height {ll}
the API additions sound fine.
the change to the calculation also sounds fine, although we may want to control 
it by the mode (quirks vs. no quirks) and as Erik alludes to, perhaps even by 
platform.  Let's get it in and get some feedback.
"me too" re- Steve comments :-)

However Erik, regarding GetEmHeight, GetEmAscent, GetEmDescent, I am not
sure that I understand you well. Since 'm' has no descender, GetEmDescent
is going to be zero? and GetEmHeight is equals to GetEmAscent?
Because they are still confusing to me, I will opt for the second option
GetNormalLineHeight, so that substracting GetLeading might give the other.

The "em square" is a term that refers to the units used in the font's design.
TrueType fonts have a field called unitsPerEm, and the glyph outlines are
defined in those units. It has no direct relationship to the letter 'm', though
its history is related to the letter 'M'. However, in modern fonts, there is
no direct relationship between the em square and the letter 'M'.
So the 'EmHeight', 'EmAscent', 'EmDescent' you are referring to here represent 
the smallest bounding-box in which all characters in that font can fit?
No, the designer is free to have some glyphs protrude outside the em square.
The em square is simply the size that you request. The bounding box is the box
the surrounds all glyphs, and it may be larger than the em square.
Checked a fix into nsHTMLReflowState.cpp.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Marking verified per last comments.
Status: RESOLVED → VERIFIED
REOPENING.

On http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight7.html one sees
that line-height: 1.0 is not using the font-size as the line-height.
URL: http://www.bath.ac.uk/%7Epy8ieh/inter...
Status: VERIFIED → REOPENED
Keywords: correctness, css1, fonts, nsbeta3, regression, testcase
QA Contact: petersen → py8ieh=bugzilla
Resolution: FIXED → ---
Whiteboard: hit during nsbeta2 standards compliance testing
Forgot to add: Tested with Windows 2000 commerical build 6.0.17.2000072811.
Blocks: 46921
mark it as Future and nsbeta3- per bug meeting w/ ekrock
Whiteboard: hit during nsbeta2 standards compliance testing → [nsbeta3-] hit during nsbeta2 standards compliance testing
Target Milestone: M16 → Future
Status: REOPENED → ASSIGNED
Summary: line-height should be multiplied by em square height {ll} → line-height should be multiplied by em square height {ll} [INLINE]
Mark it as M23 for now.
Target Milestone: Future → M23
Keywords: nsbeta3 → mozilla0.9, mozilla1.0, nsbeta1
Whiteboard: [nsbeta3-] hit during nsbeta2 standards compliance testing → [Hixie-P1] (last remaining inline box model bug)
No longer blocks: 46921
Whiteboard: [Hixie-P1] (last remaining inline box model bug) → [Hixie-P1] (most important of two remaining inline box model bug)
Mark it as moz1.0 for now.
Target Milestone: --- → mozilla1.0
erik resign. reassign all his bug to ftang for now.
Assignee: erik → ftang
Status: ASSIGNED → NEW
shanjian- this is a hard one. can you please take a look at this one ?
Assignee: ftang → shanjian
I probably shouldn't reopen this bug. Ian's observation seems unrelated with 
this bug. I traced into the program for a little while, and it seems to me that we 
did not interprete lineheight attribute correctly. In our layout code, nsHTMLReflowState.cpp,
line-height is normal. If you change the value of line height from 1.0 to 2.0, you will 
not see any difference. So the problem seems to be the interpretation of CSS.

I open a bug 82422 for lineheight problem and close this one. 

For clarification, in CSS2, it suggest to use 1.0 to 1.2 multiply the size of font to use
as normal line height. Erik suggest to use em square height here. That will produce a better
result when applying with different fonts than a fixed number.
Status: NEW → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → WORKSFORME
Why is this WORKSFORME?  This is a complicated issue where we're not sure what
the right behavior really is.  Going into the debugger won't help figure out how
we should behave.  Reopening.

INVALID *may* be an appropriate resolution for this bug, but I don't see how
WORKSFORME is since Ian's testcase doesn't work right now.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Please pay attention to my last comment. Bug 82422 is filed for ian's problem.
We would still like to handle lineheight normal using em square height. I do not 
see why I can't resolve it as worksforme. I do not like the idea of put unrelated
issues into one bug.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WORKSFORME
The correct resolution on this bug should be FIXED rather than WORKSFORME -- see
the recent comments on bug 82422.  (There was an error in Ian's test, so he
shouldn't have reopened in the first place a year ago.)  But I won't bother
reopening and correcting since that would send me and others 6 email messages
from bugzilla...
VERIFIED per comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: