Bug 80010 - CSS 2.1: height of inline boxes incorrectly excludes line gap
Summary: CSS 2.1: height of inline boxes incorrectly excludes line gap
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Robert Hogan
URL: test.csswg.org/suites/css2.1/nightly-...
Keywords:
: 80270 (view as bug list)
Depends on:
Blocks: 47141
  Show dependency treegraph
 
Reported: 2012-02-29 23:53 PST by Robert Hogan
Modified: 2014-05-31 22:19 PDT (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hogan 2012-02-29 23:53:48 PST
This bug has been around forever and shows up in the linked test from the CSS 2.1 test suite - the red background from the table cell shouldn't show through. InlineBox::logicalHeight() uses FontMetrics::height() to establish the height of the line box, but that is only the height of the text and excludes the spacing below it - resulting in a line box 1 px shorter than it should be. So it looks like it should use FontMetrics::lineSpacing() instead.

Fixing this will require an enormous number of test results to be rebaselined so may not be worth the overhead.

What do you think?
#
Comment 1 Robert Hogan 2012-03-08 22:19:58 PST
*** Bug 80270 has been marked as a duplicate of this bug. ***
Comment 2 Yi Shen 2012-03-09 08:25:09 PST
Robert, I spent some time on investigating the bug 80270, and I agree that the FontMetrics::lineSpacing() should be used instead as the logical height of the inline box. In addition, the issue only happens when the lineSpacing > ascent + descent for some fonts.
Comment 3 Dave Hyatt 2012-03-14 12:42:06 PDT
The test looks invalid to me. Line boxes should not include line gaps. The test is making the assumption that the height of the line box will equal the height of the block, and that's a bogus assumption if the font has a line gap.
Comment 4 Robert Hogan 2012-03-14 13:34:24 PDT
Filed a comment at http://test.csswg.org/shepherd/testcase/block-in-inline-001/

Not sure if that will do anything!
Comment 5 Gérard Talbot 2012-03-15 10:27:04 PDT
(In reply to comment #3)
> The test looks invalid to me. Line boxes should not include line gaps.

Hello,

I need to be extra-sure of what all of you mean by "line gaps".

A rule that uses 
table {font-size: 16px; line-height: 20px;}
will have a top-half-leading of 2px and a bottom-half-leading of 2px. 
And the line box will normally be 20px in height.

(Btw, I do see red when I add and use the above rule in Chrome 17 and then more red for topmost and bottommost cells and then more red when I increase line-height. By default, vertical-align is middle for table cells. So, by increasing the line-height value, I should see and I do see more red at the top and below "Line 1" and "Line 3")


> The test is making the assumption that the height of the line box will equal 
> the height of the block, and that's a bogus assumption if the font has a line > gap.

The inline boxes (identified with the text "Line 1" and "Line 3") may not fill the line box but the block box background (<span class="block">Line 2</span>) will.


> Filed a comment at http://test.csswg.org/shepherd/testcase/block-in-inline-001/

>Not sure if that will do anything!

I think I do understand the problem and I will propose a code correction to the test with regards to taking into consideration line-height. I have something in mind...

Gérard
Comment 6 Robert Hogan 2012-03-15 12:26:32 PDT
Thanks Gerard!
Comment 7 Gérard Talbot 2012-03-16 17:42:02 PDT
Okay. This is what I am proposing as a replacement for the test:

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/block-in-inline-001-ahemGT.html

Gérard
Comment 8 Gérard Talbot 2014-05-31 22:19:38 PDT
Hello all,

This test

http://test.csswg.org/suites/css2.1/20110323/html4/block-in-inline-001.htm

was indeed imprecise.

I have proposed

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/block-in-inline-001-ahemGT.html

and then proposed

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/block-in-inline-001.xht

as replacement. 

Somehow, the 2012-02-22 19:52:44 PST Changeset: 2772:c2fb265f7707 commit

http://hg.csswg.org/test/diff/2e6c9009b95a/approved/css2.1/src/box-display/block-in-inline-001.xht

mishandled this.

Gérard