267592 - [FIX]table column background not right when using the overflow: auto;

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect, P1)

Description

[charles allen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20041001 Firefox/0.10.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20041001 Firefox/0.10.1

background colors given to colgroup elements are not drawn correctly when the
table contains more than one TBODY element (or contains both a THEAD and a TBODY
element); AND the TBODY element(s) has the CSS property  overflow: auto;

SEE the the sample in the URL.  This happens with the overflow: auto;
property/value pair set for the TBODY element(s).

This may be related to the fix for bug 4510

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Actual Results:  
It appears to me that the background color for each child of TABLE is being
drawn from the top of TABLE, not from the top of each child.  Thus they
overwrite each other, and do not fill out the table as expected.  

Expected Results:  
In my demonstration url, I expect the last table to look basically the same as
the middle table.

Comment 1

[fantasai]

thanks for the test case :)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Simple testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: More comprehensive testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 164605 [details] [diff] [review]
Fix

When it's scrollable, the table row group is a kid of the scrollframe, not the
table frame.  So the coordinate conversions have to be handled a little more
carefully...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase using image

Comment 7

[fantasai]

Any reason why you're passing parameters for the offset instead of modifying the
calculation for rowgroups with views from within the row group function?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Because I need that offset in the "does the row group overlap the dirty rect?"
code in the table function...  I suppose I could just call GetOffsetTo() twice,
but it's not _all_ that cheap, so I figured I'd avoid it.

Comment 9

[fantasai]

Comment on attachment 164605 [details] [diff] [review]
Fix

Instead of passing the offset as a parameter, you should use the mRowGroup data
member. Set the offsets on its mRect in the calling function, then use a
SetData function (that sets what SetFull sets but doesn't touch the mRect)
instead of SetFull in the painting function.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Per comments

Comment 11

[fantasai]

Comment on attachment 166030 [details] [diff] [review]
Per comments

If you can slip
+    // Need to compute the right rect via GetOffsetTo, since the row
+    // group may not be a child of the table.
+    mRowGroup.mRect.MoveTo(rg->GetOffsetTo(aTableFrame));
under an 'if' statement so getOffset is only called when necessary, do that.
Other than that, r=fantasai :)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 166030 [details] [diff] [review]
Per comments

I thinking checking whether to call it would cost about as much as calling
it...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 166030 [details] [diff] [review]
Per comments

er, requesting sr, not setting.  ;)

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 15

[Bernd]

The more comprehensive testcase draws below the table on initial load.

Comment 16

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

Perhaps that should be a separate bug?

Comment 17

[Bernd]

I filed bug 317137 as the followup