Collapsing table borders don't paint consistently

RESOLVED WONTFIX

Status

()

Core
Layout: Tables
RESOLVED WONTFIX
13 years ago
13 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

When PaintBCBorders uses the damage rect to find the row and column to start
painting at, we get into trouble with painting looking different depending on
what the damage rect is. I think this is partly to do with spanning cells, but I
haven't fully debugged it.

Always starting damageArea at the top left of the table fixes the bug. IMHO this
optimization isn't worth it. I'll attach a patch to turn it off. If you think
the optimization is worth it, then feel free to fix the underlying problem :-).
The problem shows up in several testcases, e.g.
layout/html/tests/table/marvin/backgr_position-table.html
Created attachment 176498 [details] [diff] [review]
fix

This is a simple fix, the only question is whether it's really OK to just turn
off this optimization. Note that we still get the benefit of stopping painting
when the rows or columns go beyond the damage rect, so painting the top of a
huge table is still no problem.

I'm also correcting the erasure of verInfo to erase the whole allocated array.
Actually I suspect, looking at the code, that verInfo could just be length
damageRect.width and the last element is not currently used, but I'm not 100%
sure.
Attachment #176498 - Flags: superreview?(bzbarsky)
Attachment #176498 - Flags: review?(bernd_mozilla)
Comment on attachment 176498 [details] [diff] [review]
fix

sr=bzbarsky if bernd oks this.
Attachment #176498 - Flags: superreview?(bzbarsky) → superreview+

Comment 3

13 years ago
This will make scrolling slower. As we will not only paint a small strip but the
whole table if I understand it correctly. There is pretty much effort in the
code to get the damage area to avoid just this.

Comment 4

13 years ago
> IMHO this optimization isn't worth it.
Why do we have a paint damage area then at all in our code? 

The correct patch, instead of  hacking around without proper commenting why this
is necessary, would be to paint the table cell borders by the table cells.

If you feel that you need this for your testing, feel free to fix this, this
would really help.

I will not have time for this, as I will be busy on a foreseeable future with
crash fixes.

Comment 5

13 years ago
I mean I can help you with this but are not able to fix this by myself in a
foreseeable future. I am somehow reluctant to pass hacks into the code just to
make a test/compiler happy. If you think that you *really* need this hack
instead of a fix then please comment: 
a) that it is a hack
b) why we need this hack
c) what the proper fix would require
Comment on attachment 176498 [details] [diff] [review]
fix

alright, I don't really need this.
Attachment #176498 - Flags: review?(bernd_mozilla) → review-
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.