452964 - [FIX]:before for element with {table-layout: fixed} and {display:table} causes gaps between table cells

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Marat Tanalin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080830031750 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080830031750 Minefield/3.1b1pre

There are gaps (sort of "margins") for table-cells when tabled element has table-layout: fixed and :before pseudo-element at the same time. But real tables (not tabled div-ul-li's) are not affected with the bug.

Reproducible: Always

Steps to Reproduce:
1. Create HTML structure for pseudo table: for example, nested div [as table], ul [as table-row] and li [as table-cell]).
2. Add CSS rules for make them table-like ({display: table} for div, {display: table-row} for ul, {display: table-cell} for li).
3. Add the CSS rule for creating :before pseudo-element and set 'content' property for it like such: {content: "."; }
4. Open created page in Firefox 3.1 of version 20080818033342 or later.

OR see the testcase.
Actual Results:  
Each table cell have some visual "margins".

Expected Results:  
Table cell should "touch" siblings (as well as table edges) without any gaps regardless of :before pseudo-element existing for tabled element.

For reference (correct) rendering, see Firefox 3 or earlier or any other modern browser with :before pseudo-element and {display: table} support (Opera 9.52, Safari 3.1.2, Konqueror 4.03) or just see rendering how Firefox 3.1 renders a *real* table (reflected in the testcase) at similar conditions.

The bug is specific to Firefox 3.1.
Firefox 3.0 and earlier versions are *not* affected with this bug.

The bug exists since Gecko/20080818033342 Minefield/3.1a2pre version. Previous version (Gecko/20080817033619 Minefield/3.1a2pre) was not affected yet.

The bug probably appeared after landing the fix for the bug 238072 ("[GC] support positioned and floated generated content for non-replaced elements").

Comment 1

[Marat Tanalin]

5-th step to reproduce: specify {table-layout: fixed} rule for tabled element (div in the testcase).

Comment 2

[philippe (part-time)]

From the test case, it looks like the ::before generates an extra table-row (expected) AND additional cells on the second table-row (wrong).

Possibly related to bug 352937 or bug 148810 ?

Comment 3

[philippe (part-time)]

Attachment: testcase from reporter

Comment 4

[Marat Tanalin]

[very humbly] Any chances to get this really tremendous bug fixed in the final 3.1? Thanks.

Comment 5

[Boris Zbarsky [:bzbarsky]]

We seem to get pretty confused about the number of columns in the table is the main problem.

Note that the <table> testcase attached is quite different from the <div> one, since it inserts a separate rowgroup, not just a new row at the beginning of the existing rowgroup.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, this is fun.  We create an anonymous table row group, row, cell to contain the generated content.  We flag them all as generated content frames, because we've got that flag set in the frame constructor state.  Then we create the real table row, put it in the same rowgroup (because that's how table anon frames work), and it gets flagged as generated content too (because that flag is inherited by kids in nsFrame::Init()).  As a result everything in sight is flagged as generated content, and NeedFrameFor says yes for all those newline/tab textnodes, and we end up with way too many cells.

Comment 7

[Boris Zbarsky [:bzbarsky]]

So the fact that non-generated content ends up flagged as generated is really bad...  roc, what do you think of the idea of not flagging table pseudo-frames needed by the generated content as generated?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds good to me.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed changeset 305cbf365db2.

Comment 11

[Marat Tanalin]

Good job, guys. Thanks.

Comment 12

[Aakash Desai [:aakashd]]

Seeing that there haven't been any duplicate sightings for this issue for 3
months, I'm going to go ahead and verify this unless someone has an issue with
that.