291060 - table cells containing empty blocks have the wrong baseline

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

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

Table cells containing empty blocks have the wrong baseline.  There are also a
bunch of other related bugs -- we should write good tests for the text that will
be in the next draft of CSS 2.1.

Testcases forthcoming, eventually, but getting a bug on file so that the acid2
test bug (bug 289480) has accurate dependencies.

Comment 1

[Hixie (not reading bugmail)]

For those of you without W3C member access, the resolution that the CSS working
group made on this issues was as follows:

In CSS2.1 17.5.3, replace:

"The baseline of a cell is the baseline of the first line box in the
cell. If there is no text, the baseline is the baseline of whatever
object is displayed in the cell, or, if it has none, the bottom of the
cell box."

...with:

"The baseline of a cell is the baseline of the first in-flow line box
in the cell, or the first in-flow table-row in the cell, whichever
comes first. If there is no such line box or table-row, the baseline
is the bottom of content edge of the cell box. For the purposes of
finding a baseline, in-flow boxes with a scrolling mechanisms (see the
'overflow' property) must be considered as if scrolled to their origin
position. Note that the baseline of a cell may end up below its bottom
border, see the example below."

...and take the rest of the paragraph ("The maximum distance between
the top of the cell box and the baseline over all cells that have
'vertical-align: baseline' is used to set the baseline of the row.
Here is an example:") and put it into a new paragraph.

Then add this example at the bottom of the section:

   The cell in this example has a baseline below its bottom border,
   which leaves an empty space below the cell in its table row:

      div { height: 0; overflow: hidden; }

      <table>
       <tr>
        <td>
         <div> Test </div>
        </td>
       </tr>
      </table>

...then change "Note that if there is no cell box aligned at its
baseline, the row will not have (nor need) a baseline." to be its own
paragraph and say:

"If a row has no cell box aligned to its baseline, the baseline of
that row is the bottom content edge of the lowest cell in the row."

Comment 2

[Bernd]

this is rather a table code problem

Comment 3

[Bernd]

somewhere the buck needs to stop

Comment 4

[Bernd]

>If a row has no cell box aligned to its baseline, the baseline of
that row is the bottom content edge of the lowest cell in the row.

Ian, does that also apply to rowspanning cells?

Comment 5

[Hixie (not reading bugmail)]

The spec doesn't really cover row-spanning cells, but I think the answer is no.

Comment 6

[Bernd]

Attachment: initial patch

Comment 7

[Bernd]

Attachment: changes catched by rtest illustration

I did the large rtest and did only find 2 real visual discrepancies:
http://www.hixie.ch/tests/adhoc/css/box/table/002.html failed
http://dbaron.org/css/test/sec170201 failed
both seem like improvements to me.

Comment 8

[Bernd]

Comment on attachment 182575 [details] [diff] [review]
initial patch

David, while I am pretty sure what to do with tables there is one change to
nsBlockFrame where I am not confident.

Comment 9

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

Comment on attachment 182575 [details] [diff] [review]
initial patch

We'll probably have to revisit the nsBlockFrame changes when we implement
'inline-block'.  Could you add a comment above both of them saying so?	

Also, the "future version of CSS 2.1" is no longer future.

According to the spec text, though, an empty table has no baseline at all,
rather than its bottom, so this still isn't *quite* right.  (Ian wrote it, and
he has a very different mental model of how baselines should work than the one
our code has or the one I have.)

nsTableRowFrame::GetAscent should probably explain that it's getting the lowest
content edge, and how given the placement of the border and padding on a table
cell on the cell frame rather than the inner block frame, using the rect of the
inner block frame is correct and you don't need to subtract out the padding.

Comment 10

[Bernd]

Attachment: revised patch

This patch has only updated comments with respect to the initial one. I have to
admit that I had difficulties to parse the sentences in the review comment
before. I hope this is what the comment asked for.

Comment 11

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

So what I was trying to say in this part of comment 9:

> According to the spec text, though, an empty table has no baseline at all,
> rather than its bottom, so this still isn't *quite* right.  (Ian wrote it, and
> he has a very different mental model of how baselines should work than the one
> our code has or the one I have.)

was that this patch is incorrect in the case where a table cell begins with an
empty table.  This patch treats some part of the empty table as the baseline for
the outer cell, but the correct behavior according to Ian's text is that the
baseline should come from whatever follows the empty table.

Comment 12

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

Comment on attachment 189587 [details] [diff] [review]
revised patch

This should be closer than what we currently have, so we may as well get it
checked in, at least for 1.9.

Comment 13

[amano]

Hmm. Did that get checked in onto the trunk?

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: screenshot

this screenshot shows the difference in Acid2 test with and without this patch
(Firefox on the left is with this patch, Seamonkey on the right is today's
nightly without it)

Comment 15

[Bernd]

I checked the patch in. I opened the followup bug 311076 to address davids
comment 9