Closed Bug 317137 Opened 19 years ago Closed 13 years ago

table column background spills out on initial reflow when overflow: scroll/auto/hidden

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bernd_mozilla, Unassigned)

References

()

Details

(Keywords: testcase)

Attachments

(4 files)

The comprehensive testcase in bug 267592 does not render correctly, I guess it did when the patch was checked in.
Summary: ]table column background spills out on initial reflow using the overflow: auto; → table column background spills out on initial reflow using the overflow: auto;
So how do I reproduce it?  This worksforme with a current trunk SeaMonkey build (from Nov 15) on Linux...
Attached image screenshot
thats how it looks in ff nightly
I see the extra green at the bottom as well (FF Linux trunk).
Hmm.  I do see this, now.  The amount of spillage depends on the font-size, and at my default font-size it was only about 1px...

So I bet the issue is that we paint the background on the last row that intersects our scroll area... and that somehow that doesn't get clipped by the scrollframe.
I bet what it is is that the scrollframe doesn't clip painting of the background, since the _table_ is painting the background, not the rowgroup that's being scrolled... or something like that.  :(
I'm afraid to say that my patch in bug 317375 won't fix this. I've got table backgrounds (and collapsed borders) painting in a single display list item so we'll have the same problem as currently exists. The right way to fix this will be to shoehorn in yet another display list to catch the table row, rowgroup and cell backgrounds, and refactor nsTablePainter to have those backgrounds always be painted by their frames. I don't really want to add a seventh layer everywhere, so this will be slightly tricky.
Depends on: 317375
There's a reason why nsTablePainter paints all of the backgrounds instead of leaving them all to the individual frames. The background area isn't defined by the frame's nsRect, but by the nsRects of all the cells associated with it (including the full area of those that span into other rows/cols). If each element wanted to paint its own background, the code would have to loop through all the cells multiple times: once for each table row group, once for each col group, once for each row, once for each col, once for each cell. And looping through cells in a col group or column is not very convenient.

The code is written so that if an element has a view (which signals scrolling, iirc), it gets skipped when the table calls the painter, and the element gets told to call painter on itself when its regular Paint method gets called. If there's a particular reason why the element needs to paint itself, it can: we just have to code in the conditions. But it isn't necessary most of the time, so I don't think painting should be factored out to the individual frames.

It's been a long time since I've looked at the code, but what seems to be happening here is that the painter doesn't know how to paint column backgrounds for cells in a scrolling row group frame. I have to take off for a bit really soon, but I'll grab a nightly and take a closer look at this later tonight. The  important thing to note is that the test case sets a background on the column, which gets painted separate from, underneath, and before the scrolling row group: I don't think having the column paint itself would help in this situation anyway.
Attached file testcase with image bg
So, here's some interesting notes about the problem
  - It only applies to background color: images are not affected.
  - The overflow is related to the size of the cells: the total
    height of the painted box including the overflow is the computed
    height of the cell. In other words, the background color is being
    clipped to the scroll frame, but to cell box precision rather than
    pixel precision.
  - As mentioned in the summary, it only affects the paint that comes
    with initial reflow: painting it after scrolling it onto the screen
    for the first time doesn't trigger it, and it requires a hard reload
    to retrigger.
Behaviour has changed between
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060124 Firefox/1.6a1 ID:2006012405
and
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060126 Firefox/1.6a1 ID:2006012613
(Bug 317375 landing)
But I'm really not sure if something has been fixed or regressed or is in fact now correct.
Not fixed, you should only see the background of the body underneatch the horizontal scrollbar in the testcase. That's not happening in current trunk build which has the fix for bug 317375.
on the  dupe the testcase (https://bugzilla.mozilla.org/attachment.cgi?id=322419) only breaks for backgrounds on the table rows, table cell and tbody are OK. 
When looking at the code it seems to me that the table background painter completely ignores the fact that row groups might be wrapped in scrollable frames. In this case we should not paint outside the scrollable frame but we currently do as soon as the dirty rect is larger than the scrollable area which typically havens on the first paint where the dirty rect seems to cover just everything.
Well, I'll see if the workaround will work for me, but I am sure there are many webmasters unaware of the workaround or even of the issue...
On the duplicate of this ticket that I originally found there was a testcase
attached https://bugzilla.mozilla.org/attachment.cgi?id=311195 that is a very
good example of the issue
Keywords: regression, testcase
I knew about the workaround, but I will it be fixed in the final release of FF3?
Flags: blocking1.9.1?
Flags: blocking1.9.0.1?
Summary: table column background spills out on initial reflow using the overflow: auto; → table column background spills out on initial reflow when overflow: scroll/auto/hidden
From a web developer perspective (me), one potential workaround is to give the tbody a display:block and the thead a display:inline-block. Unfortunately this introduces other problems. I put a simple example page in my dup bug (439653):
http://sandbox.enjoybeing.net/ff3_scrolling_tbody_bug.html
My workaround for this problem was to move the background-color:xxxx style from the TRs to the TDs, and then it displays and scrolls correctly in FF3 with no bleeding.
Yes that is the "workaround", however that should not be necessary, this worked fine in Firefox 2.0.0.14 so it must be caused by something that was added to Firefox 3, there is also another issue with table rows that I am looking for the bug for.
Yes, it's a known bug with a known regression cause (bug 317375), hence why it's still open and not marked INVALID. It's great that you're interested in this bug, but please be considerate of the people CCed to this bug trying to track development changes and also developers trying to fix the bug who have to wade through useless me-too comments with no helpful information in them in order to find what is relevant.
https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
Sorry, I misread the bug with respect to what regressed it. Regardless, it doesn't need more comments agreeing that indeed it's a regression. Sorry for the spam.
(In reply to comment #22)
> My workaround for this problem was to move the background-color:xxxx style from
> the TRs to the TDs, and then it displays and scrolls correctly in FF3 with no
> bleeding.
> 

In my situation this is not possible because the tables are "striped" with alternating colors.
for alternating colors just switch the selector from the tr to the td, or example

tr.row0 { background-color:#EEE; }
tr.row1 { background-color:#DDD; }

change to:

tr.row0 td { background-color:#EEE; }
tr.row1 td { background-color:#DDD; }

tr.row0 > * {} also works if the row has a th in it somewhere.
Roc: could you let us know if this is feasible to fix on the branch? I don't understand when this regressed, as the problem was reported in 2005 but the regression range is in 2006?

Please mark as wanted1.9.0.x+ if you think this is something we should fix on the branch. There's a good number of recent dupes.
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
I don't think this is a regression. Comment #24 was retracted. I don't know why this is marked with the regression keyword.
All the duplicates claim that this used to work in Firefox 2.
I think the part that is not a regression is applying the background color to the table using col.  The part that is the regression is applying the background color to the table rows.  I didn't realize that distinction before.  The result from both looks the same and requires overflow to happen.  Roc are they both going to be fixed the same way or do we need two bugs?  
This bug's summary is clearly about table column backgrounds, and so is the testcase, so we really should have a new bug about table row backgrounds.
I reopened the first incorrect dupe: Bug 423823.
Keywords: regression
In Bug 424585 the issue is the same but in another particular case. No col element is used, the document is xhtml strict. But the cell background flows out, and also borders. Furthermore borders are fixed and can't be scrolled with table scrollbar! This happens when border-collapse=collapse. See the testcase:

https://bugzilla.mozilla.org/attachment.cgi?id=328189
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Not wanted for 1.9.0.x, but please renominate if the 1.9.1 patch is safe.
Flags: wanted1.9.0.x? → wanted1.9.0.x-
yet another testcase which is reproducable on every Platform with any FF3 version for me (not on any older Geckos!).

Relating to comment #27 and comment #22:

If style="background-color: <color>;" is defined directy to tableRow element and overflow is enabled, the background color will bleed the bottom.
But if the row is styled by an CSS class, it doesn't!

see attached testcases bleedingAltColors.htm and NoBleedingAltColorsWithClass.htm

This is very annoying to some implementations and should be fixed in stable tree 1.9.0.x too!
Marcel the table row problem is not this bug.  That's bug 423823 which is fixed in the upcoming FF 3.0.2.
Depends on: 28800
Is there a bug here anymore (FF4+)?  I see no problem with the testcases here or in bug 267592.
WFM even with font-size = 72 (Comment 4)
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
fixed by the patch in bug 28800 by remvoing the scroll frame.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: