dynamic changes in border-collapse tables don't invalidate enough




14 years ago
10 years ago


(Reporter: frnchfrgg, Unassigned)


({css2, testcase})

css2, testcase

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [patch])


(4 attachments, 4 obsolete attachments)



14 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041007 Debian/1.7.3-5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041007 Debian/1.7.3-5

I had a lot of surprising problems when I designed sites with the
border-collapse:collapse property enabled (this is the default).

In fact there are a lot of tiny bug reports for several problems I encountered
(see 262560, 244135, 248374, 155955), but I think it is a deeper problem.

It is not specific to a dynamic change of CSS styles, or classes, because I get
wrong behaviour even in a static case (but in the dynamic case several errors
show up).

It seems
a) That invalidate rect covers only the space belonging to the table, that is to
say half of the borders are out of it, but only in the case of a CSS border
conflict which is solver in point 4 of border conflict resolution as specified
in the CSS2 recommendation

b) That rects at the intersection of a horiz border and a vert border do not
ever get the right color (the border conflict resolution algorithm sets its
color as well as the neighbouring 'preferred' border color). There are also
'missing' (with gray color) borders.

c) That sometimes the selection of a color for a border influences the whole
line or column border (as in the second table of the example, when you put your
mouse under the big bordered row)... Very strange.
[ONLY PRODUCED IN DYNAMIC, do not know if specific or not]

d) When dynamically changing class (and thus style), border-width seems not to
be affected. It could (as far as I know) be a FEATURE, because CSS
recommendation does not tell what to do in such a case, and one would surely
avoid calculating table layout again, but...

e) Some times, when we change from a black border to a gray one, a black piece
of border appears ! In fact, 'missing' borders are drawn at that time.
It seems to occur only when we move the mouse to the cell above (and below ?).

I am sure a lot of other errors can be found using this test page, which seems
to gather every erroneous choices Gecko makes :-((( And there are shomewhat few :-)

Reproducible: Always
Steps to Reproduce:
1. Open the test page (xhtml, but not specific, tried in legacy HTML mode too)
2. That is already sufficient to see some of the errors (b in particular)
3. Move your mouse on the tables to see other errors

Actual Results:  
I saw completely erratic rendering of borders, described in detail... in the
details section ;-P

Expected Results:  
'highlighted' rows should be in bgcolor yellow and border black, with (depending
of the choice of implementation) should (or not) be 5px wider in the second
table only.

Comment 1

14 years ago
Sorry if this bug report is repeating information found in other bug reports.
But I think it adds some.

Comment 2

14 years ago
Created attachment 167037 [details]

Here is the test xhtml page I discuss about in the details section

Comment 3

14 years ago
I tested a bit more, and that's what I found.

When a row's class is changed (and thus CSS style of it's tds),
the invalidate rect on the page is limited to half of the border.
Indeed, only half of the border really belongs to the row, and the other half is
on neighbouring cells.

That's why when we do dynamic changes some colors remain unchanged, and also why
some appear at the wrong time !

For example : I am on a HL row and move the mouse down to the cell just below.
Several cases can arise :
- The first (upper) row is invalidated (because it looses its HL class) after
the second gains its HL class ; then the upper part of the border beetween the
two rows becomes black. The lower part become black when the below row is
invalidated. Then the whole border is invalidated, though it really is a

So we should invalidate a bit more than the row's extent, namely the rect
including whole borders. Then some borders may be invalidated twice, when the
mouse leaves a row and when it enters the row below, but we cannot predict next
position of the mouse, so it is the right behaviour

It is one of the problems I encountered and can surely be solved easily [and may
perhaps have, I use a standard release build so cannot know if the problem still
exists in latest builds]. Solving this sole problem can bring much more
correctness to the collapsed-border table rendering.


Comment 4

14 years ago
Concerning the 'bug' with borders not changing their width dynamicly, It IS a
bug, because in the separate borders model Gecko's crew has decided to update
border widths as well (which can lead to much flickering in a bad designed site).

BUT, the fact that widths are changed in CSS - even if the change is not drawn -
prevents borders to be half-invalidated. [It seems so at least]

There remains problems, as whole line border drawn in black, ..., but this one

I so question myself : How is the rendering of border-collapse: collapse tables
written ? Why invalidates are different if the border-size has changed ?

Shouldn't there be a single rendering engine that
1) solves conflicts - and this part is currently wrong - and decide which will
be the properties of every table's part
2) Decides what portion of the screen must be invalidated, in comparing old and
new decided layout ?

I have currently no time to look at the code, but I may do so. Meanwhile, I
refer to you far more skilled folks.


P.S.: Sorry for so much updates in so short delays ; I know some people will get
upset by my trend to fill up their mail boxes.


14 years ago
Keywords: css2, testcase


14 years ago
Attachment #167037 - Attachment is obsolete: true

Comment 5

14 years ago
Created attachment 167122 [details]
New version of the testcase

- To be able to test CSS choices without the invalidate of interferences, class
HL is never removed for first table.
- HL elements in second table have a wider border (notice that only the one
that got HL during launch gets this border).
- Second row of third table has a wider border.
- Remaining tables are alike the first one, but with initial HL changed.

Comment 6

14 years ago

Depending on the way we manualy invalidate the table, we get differents results :
- When it is fully invalidated, either because of a change of the border width
in CSS [which seems to, while not really changing widths of borders, invalidate
the whole table] or because of a full screen redraw (tab switch, window
reduction/restore, workspace change, ...), then we notice problems already
described, as blackening of the full vertical border instead of just the part
adjacent to the HL cell.
- When we invalidate parts by scrolling the table, it becomes erratic : some
times we get the same result as when we invalidate everybody (which is expected,
because we want to get always the same result, even if it is wrong because of a
previously ill-taken decision), and sometimes we get gray borders.

There so are two profound problems in the rendering :

1) Invalidates are rarely done as they should, with the puzzling case of partial
scroll-related redraw.
2) Choices of who to put in what color/size are wrong if we refer to the CSS2

Try every case described with the testcase that should, now, work (I had sent it
as text/html, so <!CDATA[[ ]]> part was ignored and function 'hi' undefined),
and see. You can also use the same testcase with 'border-collapse: separate' to
notice the correct behaviour in that case (WITH border changing).

It's better to limit bug reports to one issue each, so that issues can be
discussed separately, simpler testcases can be added, bug status can be tracked,
Although it seems, from reading the 5 points, that this does cover primarily one

Since this mostly covers failure to invalidate, I'll make this bug about that
(although that bug summary may still be too broad).  Bugs in static rendering
should be filed as separate bugs.
Summary: Gecko's rendering of 'border-collapse: collapse' is completely broken. → dynamic changes in border-collapse tables don't invalidate enough

Comment 9

14 years ago
(In reply to comment #8)
> Although it seems, from reading the 5 points, that this does cover primarily one
> issue.
> Since this mostly covers failure to invalidate, I'll make this bug about that
> (although that bug summary may still be too broad).  Bugs in static rendering
> should be filed as separate bugs.

In fact, I primarily posted this bug report because I found a lot of tiny bug
reports all describing single errors in the engine rendering collapsed bordered
tables, and because there were a LOT of them I wondered if a more profound
rewrite would be helpful... But I know this is a great amount of work to do so
and that people able to do that are the happy few.

While examining errors, I found there is mainly a well circled invalidate bug,
and another problem*. We could fork this thread in two if you want.

* it really seems that Gecko does not manage CSS priorities correctly in this
case ; sometimes the border is blackened from one extent of the table to another
instead of just around the cell, sometimes each border of the cell has not the
same color - though the color of all of them is changed in the same rule.
Perhaps Mozilla's decision is 'too' local and he makes different decisions at
different times. The solution is perhaps to maintain a set of properties for
EACH border of the table, which may permit to solve conflicts in a W3C manner,
and compare between two successive states to know what to invalidate.

See comments #4 & #6.

Do whatever you want with my reports, I only want to help designing a better
engine (hard task :-) and I am willing to adapt myself to your habits.

I'm not following any issues other than invalidation that you think are wrong. 
The testcase is complicated, and understanding what you think is wrong requires
a *clear* description of what the expected behavior is and how Mozilla's
behavior differs from that behavior.  (In other words, saying something like
"the left border of the third cell in the second row of the fifth table is
black, but it should be gray.  This appears to be because Mozilla is treating
the column border as higher priority than the cell border".)  There's not room
for more than one of those in a bug without massive confusion about what comment
is about what problem, so they should be in separate bugs.

Comment 11

14 years ago
The 'solution' I propose for the rendering -- if it is usable -- may probably
solve both 'sub'-bugs I submitted in this thread. So it may be logical to
discuss both in the same.

I restate I do not know how it is implemented, somebody has certainly found a
far cleverer way to do it, but it is currently broken. I am not sure fixing each
error with a local patch may lead to fast and reliable code -- when I develop
software (which is far simpler than Gecko I must admit) and it happens that
there are a lot of bugs on the same unit of sense, I prefer planning a ground
rewrite of the erroneous part, because most of the time such bugs are
inter-related (but I write smaller software, and in this case it may be
unfeasible to restart even the sole collapsed border table rendering).


P.S.: I'll try in the future not so split what I say in two posts. Sorry.

Comment 12

14 years ago
I can start a new bug-report for the other problem I noticed (apart from

The testcase is not that complicated. There are five tables, to really find out
what are the causes of what.

The invalidation problem is blattant, but does not occur in second table. The
only difference between it and the others is that I order (but am not obeyed) a
change of border-width. It thus seems that the invalidating error is related to
moments when nothing but the color changes (at least not the border width).

What I state is that even if we do not see it (because of the wrong invalidation
problem) there are sometimes borders that are drawn in black whereas they shouldn't.

Further explanation :
When the first two rows of the first table are HL, whatever CSS tells us about
how to render them, the other rows MUST remain all borders grayed, and that
clearly isn't. That's why I disabled the lost of HL class for the first table,
in order to be able to force invalidation in a couple of manners.

Comment 13

14 years ago
The example of error you gave in comment #10 is indeed a bug I encountered.
("the left border of the third cell in the second row of the fifth table is
black, but it should be gray.  This appears to be because Mozilla is treating
the column border as higher priority than the cell border".)

Created attachment 167141 [details] [diff] [review]
fix invalidation problems

This fixes the invalidation problems by including a large estimate of the
collapsed border in the overflow area.
Comment on attachment 167141 [details] [diff] [review]
fix invalidation problems

This is sort of ugly, but it fixes the repainting problems, although there are
still a bunch of other problems.
Attachment #167141 - Flags: review?(bernd.mielke)

Comment 16

14 years ago
(In reply to comment #15)
> (From update of attachment 167141 [details] [diff] [review])
> This is sort of ugly, but it fixes the repainting problems, although there are
> still a bunch of other problems.

I read the patch you proposed, and though a bit ad-hoc (a too local solution) I
think it solves this problem of invalidates.

Could you yet try and see if you can get this wrong behaviour :

I load the testcase, and highlight the second row of the first table by moving
mouse over it. Then I force a total redraw of the screen and get a first error :
the whole left and right borders of the table get painted in black.
Then, I scroll down to hide the first table, and scroll up to reshow it.
At that time results I get are unpredictable.
I get a mix of the table whose left & right borders are gray and the table with
the same borders black. In other words, repainting does not produce the same
result each time.
I am not even sure one of the renderings (the one when I get gray borders) is
right, but I am sure of one thing : if it is the same repainting method which is
called when we do a full repaint and a partial repaint due to scrolling, it's
worth turning everybody crazy.

I'll attach a image of what I get.

Comment 17

14 years ago
Created attachment 167143 [details]
Screenshot of an example of what I can get

Comment 18

14 years ago
Hmmm, at first glance I can't believe that this patch is correct.

The computed border styles for BC are stored in the cellmap and not in the table
cell style properties. The style properties would need to be an array for this
as one bc table cell border can have more than one segment due to col- and rowspans.

The current bc computation stores in the cellmap the information whether a
corner is a start for a segment. Color changes mean different segments. Each
border change, including a color change in the current scheme needs to cause
either a style change reflow of the corresponding bc table descendendants  which
should then set the BC damage Area (bug 229712) or set the BC damage area
directly to force a recomputation directly. (yes, I know this is a hack and even
worse its hack for a hack)

The things I'm using for the invalidation are a maximum for the inner part, set
by nsTableFrame during border computation.  Some other things seem to work
right, modulo invalidation (done by ApplyRenderingChangeToTree) not invalidating
what's needed, but a bunch of things also don't work.  If you want this to work
some other way, you'll need to implement a new style hint.

Comment 20

14 years ago
I think the model of rendering should be this one in collapsed border tables :

Wa maintain a private member which is a two-dimmensionnal array of border

Every chunk of border is referenced (even when we have colspans or row spans)
but we could have a special value meaning that we have the same properties than
another specified chunk. Each cell in the table could have a member telling us
which chunks belongs to it.

Then, when we have a style change (due to whichever reason), we can compute
again a new array of border properties according to CSS conflicts resolution,
and then we can in each cell know what to invalidate, and the redraw is easy to
do -- and fast, because every decision has already been taken.

To know what to invalidate during the change of style, that is when we discard
old border-props array and store the new one, it is easy to compare the old an
new border array and invalidate each chunk that need it [while not forgetting to
reflow the whole table if a border-width has changed -- unless we choose not to
update border widths during dynamic CSS changes, to avoid too much flickering ;
it is not, though, the choice made in the non-collapsed border model]

I am pretty sure it would be efficient and easy to implement -- perhaps it is
already done this way and then errors would be easy to track and eliminate.


P.S.: Rewriting only the collapsed border model seems to be a matter of
reimplementation of nsBCTable*Frame classes, so it may be feasible.

Comment 21

14 years ago
Comment on attachment 167141 [details] [diff] [review]
fix invalidation problems

Including only the cell border is correct, if it is the widest and wins, then
we will invalidate wide enough if it is too small or does otherwise not win
then changin the color will have no effect anyway. What I like with this patch
we make a step towards painting the border by the cells themself.
Attachment #167141 - Flags: review?(bernd.mielke) → review+

Comment 22

14 years ago
I also like the idea of painting borders directly by concerned cells.

But the decision of which color/size/etc... to put on a particular border cannot
be taken by a single cell, that's why I spoke about a pointer to each of four
borders adjacent to the cell.

I should download and discover how the engine of border collapse model is
written -- or you could tell me (briefly) what is it's structure.


Comment 23

14 years ago
The general problem with current BC code is outlined in bug 203686. Inside
CalcBCBorders we compute the borders and store them in the cellmap (this is the
2 dim array you are talking about see
Getting a working debug build is always a good idea. Just ask me by email if you
ahve further questions.

Comment 24

14 years ago
I read source code from the link you submitted to me.

I wonder if the strategy for border collaped tables painting is really good,
because borders overlapp cells, so painting must be done in a specific order,
which is not necessarily the order of the paint events.

I think the table must be responsible of the drawing of everything of it that is
OUTSIDE cells. That means nothing gets painted in cells by the table, and cells
are to paint their halves of borders. This way we have no problem about
repaints, and I think there won't be an enormous downgrade of speed introduced
by the two-steps borders repainting.

Or, we could consider that cells are responsible for drawing their full borders,
 but then we could redraw the same border twice, so we may not gain much
concerning speed.

Current solution is to my mind a hack of the non collapsed model, and we
shouldn't rely on that rather than writing a separate engine for a separate model.


Comment 25

14 years ago
A precision to last comment :

There is no problem for other thinks drawn by the table than borders, because
only borders must stay over cells. Thus when I say 'the table paints nothing
inside cell space' you must read 'no border'.


Comment 26

14 years ago
Is it normal that this bug is still UNCONFIRMED whereas it has successfully been
reproduced on several systems ?


Comment 27

14 years ago
Created attachment 177480 [details]
Minimal testcase for the bug (and another).

I still can get the invalidation bug on Firefox 1.0.1 (debian build).
Here is a minimal testcase, that also underlines another blattant bug (one of
those I already talked about). I'll file in a bug report for this one too, if
there is none already.

Attachment #167122 - Attachment is obsolete: true
Attachment #167143 - Attachment is obsolete: true

Comment 28

14 years ago
Created attachment 177482 [details]
Same testcase, with :hover trigger


14 years ago
OS: Linux → All

Comment 29

14 years ago
This bug has been reproduced on Mozilla & Firefox, Windows & Linux.

I do not have a Mac lying around, so I couldn't check.

But please do not leave this bug UNCONFIRMED, as nobody takes it into account.
-- I am not empowered enough to do so bugzilla tells me;(.

Comment on attachment 177591 [details] [diff] [review]
fix invalidation problems

We need a comment describing GetSelfOverflow. 

I also think that nsBCTableCellFrame::GetSelfOverflow needs an XXX comment
noting that this is potentially going to throw outlines and scrollarea sizes
off by one pixel.
Attachment #177591 - Flags: superreview?(roc) → superreview+
I filed Bug 286794 about a regression caused by this bug here.

Comment 34

14 years ago
My girlfriend was playing with the testcase, and discovered some other stuff :
she realized she could do sort of a painting with a window invalidating regions
of the viewport.

And I noticed one thing : The border of the cell, which should be yellow, is
always drawn yellow correctly (when and where it is asked to be redrawn, of
course, but your patch solves that). But the part of the borders wrongly painted
in yellow aren't painted yellow every time.

In fact, I have a far more precise information and have filed a new bug for that
: "https://bugzilla.mozilla.org/show_bug.cgi?id=286797"

Comment 35

14 years ago
Isnt this bug fixed by bug 286794?
The regression from the invalidation patch here was; this bug still describes a
whole bunch of other problems (see comment 7, etc.).

Comment 37

14 years ago
(In reply to comment #36)
> The regression from the invalidation patch here was; this bug still describes a
> whole bunch of other problems (see comment 7, etc.).

Yes, but I filed another bug
(https://bugzilla.mozilla.org/show_bug.cgi?id=286797) only on the other problems
encountered, with a minimal testcase (far more simpler than the one that is
here) and a precise description of the behaviour (I namely found that the wrong
color was painted if and only if the invalidate region intersected a given pair
of rects, see tescase for details).

The remaining bug here is only the invalidation not covering the whole borders,
and it isn't solved in FireFox 1.0.2. But I gathered from previous comments that
the proposed patch broke some other behaviour, so this may explain that.


P.S.: I'd really like some folk to examine my other bug, because it is far more
precise. Moreover, nobody commented it, nobody confirmed it (apart from friends
of mine, but not powered enough to mark the bug new). You advised me to split my
bug in two, and I did so, but I expect it doesn't mean that a half would be
paused an indefinite amount of time. (I am not upset, nor angry, nor agressive,
I just express a wish).

Comment 38

14 years ago
_FrnchFrgg_ - have you asked Gerv for upgraded permissions yet?
The testcase there *still* seems to cover about 4 different things.  The
testcase for the bug should cover *one* problem; if you think the others are
likely to be fixed by the fix, you can also add them as additional testcases to
the bug, but the primary testcase should be something that either passes or fails.
(Maybe it's not so bad, actually, but it's still better to have the steps to
reproduce in the bug, outside the testcase, and with clearer expected and actual

Comment 41

14 years ago
(In reply to comment #40)
> (Maybe it's not so bad, actually, but it's still better to have the steps to
> reproduce in the bug, outside the testcase, and with clearer expected and actual
> results.)

The testcase covers mainly one problem, that is the invalidation only on the
inner part of the cell's border. I thought I had to mention the other bug
because it appears when you manually invalidate the whole viewport, and I had to
describe what happened. In fact, the supplied testcase doesn't speak much about
this second bug, whereas it is wholly described in its own bug, with a lot of

For the record:
 - Expected results: When the page is loaded, table borders should be wide and
black. Then when the color is dynamicaly changed, either by javascript when
clicking on the body (testcase 1) or by CSS pseudo-class :hover (testcase 2),
the color of the border enclosing left cell should become yellow.
 - Actual results: Only the part of the border that is over the cell is
repainted, because the invalidate rect is too small (manually invalidating shows
the right color, apart from strange behaviour that is subject of another bug and
won't be solved by only correcting the invalidate rect, for sure)


Comment 42

14 years ago
This bug seems to be corrected in the Deer Park Alpha 1.

There remains the other bug, but it is already far better.


Comment 43

13 years ago
Isn't this bug FIXED in FF 1.5 and current trunk ? I could set resolution since I am the reporter, but I am not sure I should play with such things...
There are still a bunch of problems with borders remaining in the wrong state when mousing over attachment 167037 [details] moving the mouse up and down over the tables, and also a weird problem in the second table when moving the mouse from the second row to the third.  Those should be filed as separate bugs before this one is closed.  (That's one of the reasons I wanted one issue per bug originally.)

Comment 45

13 years ago
(In reply to comment #44)
> There are still a bunch of problems with borders remaining in the wrong state
> when mousing over attachment 167037 [details] [edit] moving the mouse up and down over the
> tables, and also a weird problem in the second table when moving the mouse from
> the second row to the third.  Those should be filed as separate bugs before
> this one is closed.  (That's one of the reasons I wanted one issue per bug
> originally.)

To me, those seem all to relate to bug 286797.

Comment 46

13 years ago
More precisely, if there are other problems than the one described in bug 286797, there will be very unlikely to be decipherable until 286797 is solved. Perhaps keep the bug open to still have the "monster testcase" handy...

Comment 47

13 years ago
I think that the reason of bug 286797 is that the style change does not trigger any update of the border map. By adding

nsRect rect(0, 0, GetColCount(), GetRowCount());

at the beginning of nsTableFrame::PaintBCBorders, thus forcing the border map to always be up to date by a ugly hack, the testcases (mostly) behave.

Remaining gliches of monster testcase :
- static one, outlined in bug 286797, should (and may already) have its own bug. Note that when 286797 is solved, the dynamic case will get the same (wrong) behaviour as the current static case.
- some strange changes of size of cells in the second table, not outright wrong, but unpleasant to the eye. May come from my ugly hack.

Comment 48

13 years ago
Static case bug is bug 325074.

Comment 49

11 years ago
Is there any improvement on the border rendering coming? It´s 2 years ago now, and even in version 3.0b2 it´s still the same: Table borders cannot be drawn dynamically in collapse mode. This is very very nasty and prevents many designs which are no problem in Internet Explorer!

Even with collapse=separate the rendering is not robust and sometimes stochastical.

I can´t believe that this is such a big issue that it cannot be solved within 4 years.

Best regards,

Comment 50

11 years ago
Kristian: The table border code is an ugly mess, and nobody has enough time amongst those skilled enough to rewrite it. Those happy few are... few, and their time has been spent evolving other parts of Gecko.

Comment 51

11 years ago
That´s what I expected, but the hope dies last ;) Thanks for your response.
The "mess" applies to Thunderbird also, by the way. There´s so much ... okay, let´s see what happens in further versions.

For the certain case I have found a workaround - when I create the complete table definition by document.write, it works.

Best regards,

Comment 52

11 years ago
Created attachment 322785 [details]
Additional testcase for dynamic changes, including workaround

I've run into this bug myself on both Firefox and Firefox 3.0-rc1. The attached testcase demonstrates a case where Firefox renders the borders wrong after a dynamic change. Additionally, it includes workaround code (tick the "Apply workaround" checkbox, basically toggles border-collapse after changing borders to force recalculation/repaint), which allows the dynamic changes to be applied correctly.

To see the bug, mouse over each of the groups on the left. To see the correct rendering, tick the checkbox and then mouse over the groups again. Furthermore, the rendering is differently incorrect depending on the initial correct state (that is, if the first, second, or third row is selected and drawn correctly).

There is a broken image at the right, this is intended, it's just a placeholder (and has an empty src).

Comment 53

10 years ago
Julien, isn't this bug wfm now?

Comment 54

10 years ago
(In reply to comment #53)
> Julien, isn't this bug wfm now?

See comment 42 to comment 46, and comment 48. Short story: the invalidation bug has been fixed, and I believe that the other bugs exhibited by the "monster testcase" (attachment 167037 [details]) are bug 325074 and bug 286797. I think we should close this bug, but it may be a good idea to keep the "monster testcase" around to really verify if it doesn't exhibit another problem after the others are fixed.

Comment 55

10 years ago
Just to be clear, the resolution should be FIXED and not WFM, since the bug has been fixed here by attachment 177591 [details] [diff] [review].


10 years ago
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.