15428 - [TEXT] line-height values less than 1 are wrong {ll}

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Masatoshi Kimura [:emk]]

To reproduce:
1. Launch apprunner.
2. Nevigate to http://member.nifty.ne.jp/emk/moz/line-height.html

Expected result:
The lines overlap each other as if only one line exists.
IE5 and Nav4 works as expected.

Actual result:
The lines do not overlap completely.
I'm using [1999100118] build on Windows NT 4.0 (SP5)

Comment 1

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

One could argue that this shouldn't work.  See bug 4234 and bug 13072.

Comment 2

[Masatoshi Kimura [:emk]]

I read bug 4234 and bug 13072, but I couldn't understand why this shouldn't
work.

Comment 3

[kipp]

Interesting bug.

According to the css2 spec (and numerous email discussions), text isn't an
element and therefore it doesn't contribute to line-height calculations.
Therefore, we should be able to end up with a zero height line given that there
are no elements provided in the line that have a height and therefore the blocks
line-height (== minimum line-height) is coming into play.

Yes?

Comment 4

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

The problem is that line-height is based on the computed font-size, which is
*almost never* the actual font size because of messed up fonts.  The way you do
things, which is arguably correct (since it makes line-height: 1 work
correctly), is to get the half-leading from (font-size - line-height) [computed]
and then put the half-leading around the edges of each inline box.

In order to fix this, you'd have to disobey the probably-incorrect wording of
the spec and base the actual value of the line-height (when a scaling-factor,
anyway) on the actual value of the font-size, and then use both actual values
for the leading computations.

Then comes the question:  do you do the same for the 'em'?

(Or you could look at the whole thing as what you're currently doing is using
actual font-size and computed line-height and ignore the half-leading bit, or
something like that...)

I've raised the issue of how bad OS font-sizes should be accomodated on
www-style, but nobody seemed receptive to making any official statement.  And I
don't know if the actual/computed issues on line-height are going to be
clarified anytime soon either.

line-height: 0 should work with the "Ahem" font. :-)

The usual references:
http://www.fas.harvard.edu/~dbaron/css/fonts/sizes/
http://www.fas.harvard.edu/~dbaron/css/fonts/explanation
http://www.fas.harvard.edu/~dbaron/css/fonts/aspect_results
http://lists.w3.org/Archives/Public/www-style/1999May/0026.html
http://lists.w3.org/Archives/Public/www-style/1999May/0036.html

Comment 5

[kipp]

I'm currently computing the line-height value based on the actual font-size as
opposed to the value defined by the style system. EM units, on the other hand,
recieve no special treatment and are therefore inconsistent :-(

Comment 6

[kipp]

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

Comment 7

[Hixie (not reading bugmail)]

We seem to be doing strange things indeed for line-height: 0. In fact we are
doing weird things for all line-height values less than 1.

Sections 5 and 6 of the Evil Tests "lineheight3" contain many thorough, complex
test cases which should probably be distilled and turned into small tests.
   http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/lineheight3.html
These tests use the Ahem font.

[David: you may want to verify my mathematics...]

Comment 8

[Masatoshi Kimura [:emk]]

Positive lfHeight version also causes this bug.
Probably this bug is not related with actual vs. computed height
since both actual and computed height are the same on positive
lfHeight version.

Comment 9

[Masatoshi Kimura [:emk]]

Here is the problem:

1. When nsLineLayout::VerticalAlignFrames is handling the root span,
   the initial values for the minY and maxY are zero so that only the
   child frames will impact their values.
   But this is not enough to avoid impact since yBottom for the child
   frames may become a negative value when line-height is shorter
   than font-size.

2. The BR frames lie on the baseline since BRFrame::Reflow set the
   aMetrics.ascent for them.
   But other frames assigned 'line-height:0' will be above the
   baseline, so the distance between the uppermost box top and the
   lowermost box bottom will not be zero.

Comment 10

[leger]

Updating to default Layout Assignee...kipp no longer with us :-(

Comment 11

[troy]

Why are you re-reassing layout bugs? Do NOT touch layout bugs.

The bugs are assigned to Kipp so they can stay neatly organized until we have a
new owner for the block/inline code.

Comment 12

[Hixie (not reading bugmail)]

VYV03354: Very nice work on finding the cause! :-)

So... what would the solution be?

Comment 13

[buster]

mass moving all Kipp's pre-beta bugs to M15.  Nisheeth and I will
prioritize these and selectively move high-priority bugs into M13 and M14.

Comment 14

[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 15

[ekrock's old account (dead)]

Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!

Comment 16

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

I have the fix for most of this bug in my tree, although it still doesn't work
for lines terminated by BR.  BR needs to be fixed.

Comment 17

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

I think what's left is that BR frames need to be made so that they set their
size and position based on line-height and font metrics (rather than set their
position to height 0 on the baseline, which can cause expansion of narrow
lines).  I think it is best to make it so BR frames are ignored in compat mode
(which will be more important after the above fix), but this will still matter
in strict mode.  I am working on the ignoring code now...

Comment 18

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

Attachment: additional testcase, quirks mode

Comment 19

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

Attachment: additional testcase, strict mode

Comment 20

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

In the above testcases, the first test tests a line-height of 0 (with and
without BR) and the second tests BR with a line-height of 3.

Comment 21

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

OK... I've got this one fixed too (I made the BR element obey line-height).

Assigning to myself.

Comment 22

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

Fix checked in 2000-02-14 20:26PDT.

Comment 23

[Hixie (not reading bugmail)]

David: Wow. All relevant tests passed with Viewer in 2000021509 build.
VERIFIED FIXED on Win32.