195291 - opacity, fixed backgrounds on tables paint wrong

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[fantasai]

Table cells are not being painted completely when affected by opacity or a fixed
background. Some of them paint the height they'd have if other cells did not
affect the row height; the ones in the row with a rowspanning cell disappear.

Comment 1

[fantasai]

Attachment: testcase

Comment 2

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

Most likely this is tables not syncing views with frames properly after a cell
size change.

Comment 3

[fantasai]

What does "not syncing views with frames" mean?
Opacity and background in the testcase are specified on the table row group
here. Why would different cells in the same row group paint differently? Does
the row group Paint get to paint its own children or do they have views, too,
and get painted by something else?

Comment 4

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

I dunno, I'll have to read the table code.

Comment 5

[Bernd]

>I dunno, I'll have to read the table code

You are very wellcome, could you point in the meantime how it is done for
blockframes. I guess there is no documentation on this topic :-(.

Comment 6

[fantasai]

Table elements paint their children if the children don't have views. So, unless
the cells have views, calling Paint on a table row will also paint all the cells
in that row. I don't think the table code is doing anything else with the
views--although Bernd would know better than I. The Paint functions certainly don't.

What does "not syncing views with frames" mean?

Comment 7

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

This should help:

http://ocallahan.org/mozilla/view-talk

Comment 8

[fantasai]

Yes, that does help a bit.

The only views handling seems to be

366 // Make sure any views are positioned properly
367 void
368 nsTableFrame::RePositionViews(nsIPresContext* aPresContext,
369                               nsIFrame*       aFrame)
370 {
371   nsContainerFrame::PositionFrameView(aPresContext, aFrame);
372   nsContainerFrame::PositionChildViews(aPresContext, aFrame);
373 }

It's called on table elements' children. (table, row group, row all call this)

From nsTableCell.cpp:

686   if (kidYTop != kidRect.y) {
687     // Make sure any child views are correctly positioned. We know the inner
table
688     // cell won't have a view
689     nsContainerFrame::PositionChildViews(aPresContext, firstKid);
690   }

I'm not sure what "inner table cell won't have a view" means--Bernd?

So, roc, do we need to add other stuff here? 

Comment 9

[Greg Wogan-Browne]

This used to work for me in 1.1 (possibly 1.2 I'm not sure now) but seemed to
die around the time bug 119070 mysteriously became fixed. It seems a similar
issue (tables can't do opacity for ****).

Comment 10

[José Jeria]

Isnt this a dupe of bug 193790? 

Comment 11

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

Can you isolate this to particular kinds of tables?

After moving or sizing a frame you should always call
nsContainerFrame::SyncFrameView (or better still,
nsContainerFrame::FinishReflowChild, which does a few things including
SyncFrameView). A quick glance seems to show that
nsTableRowFrame::ReflowChildren does call FinishReflowChild for table cells, but
there could well be other places that resize child frames.

Running viewer, I can see that TH-A's view is sized to 323 twips high (text
height?), but the the frame is resized to 765 twips high (vertical alignment
code?). This mismatch is definitely the problem. If you show me where the cell
is resized to the height of the row then we can likely find the problem...

Comment 12

[Martijn Wargers (dead)]

Attachment: Simple test-case to show when -moz-opacity cuts of table cells

Maybe this one helps.

Comment 13

[Asa Dotzler [:asa]]

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

Comment 14

[Bernd]

The patch in bug 230730 fixes the testcase, the second at least, I am not sure
what is the correct rendering of the first test.

Comment 15

[Bernd]

fixed by bug 230730