table borders disappear occasionally with border-collapse and rowspan

RESOLVED FIXED

Status

()

Core
Layout: Tables
RESOLVED FIXED
13 years ago
3 years ago

People

(Reporter: Gary Shi, Assigned: Malcolm Parsons)

Tracking

(Depends on: 1 bug, {testcase})

Trunk
x86
All
testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
blocking1.8.1 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: ["Patch to fix the damage area" needs checkin], URL)

Attachments

(11 attachments, 4 obsolete attachments)

1.28 KB, text/html
Details
12.78 KB, image/png
Details
248.75 KB, text/html
Details
11.74 KB, image/png
Details
34.83 KB, text/plain
Details
20.76 KB, text/plain
Details
8.96 KB, patch
Details | Diff | Splinter Review
725 bytes, text/html
Details
39.31 KB, text/plain
Details
4.31 KB, patch
Details | Diff | Splinter Review
10.05 KB, application/xhtml+xml
Details
(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

We're going to make all the borders shown as a solid line in the main table on
that page. It works without any problems when the table is small (i.e., around
15 rows) or we don't specify "ALIGN='center'" or "STYLE='text-align: center'" in
<TR> or <TD> element. But when it grows larger, some of the lines disappears on
some of our computers running Mozilla 1.6 or Firefox 0.8. It's always ok when we
save the page on local disk and open it locally.

Reproducible: Sometimes
Steps to Reproduce:
1. load http://202.109.110.179/~garyshi/register.htm
2. if the table displays all the borders, reload it.
3. usually you will see a wrong render within 5 loads.

Actual Results:  
screenshot at http://202.109.110.179/~garyshi/reg2.png

Expected Results:  
screenshot at http://202.109.110.179/~garyshi/reg1.png
Created attachment 148947 [details]
Testcase which uses the Maciej/Hyatt hack to trigger the bug

From that page, I've made a testcase which does show the problem 100% of the
time by using the Maciej/Hyatt hack. This triggers an early reflow, see:
http://weblogs.mozillazine.org/hyatt/archives/2003_08.html#003963

To get the bug:
    * The table must have border-collapse:collapse
    * The table must have a table-cell with rowspan=4

Comment 2

13 years ago
confirming firefox trunk 20040522
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase

Comment 3

13 years ago
*** Bug 257371 has been marked as a duplicate of this bug. ***

Comment 4

13 years ago
*** Bug 271607 has been marked as a duplicate of this bug. ***

Comment 5

13 years ago
FIX THIS!

Comment 6

12 years ago
http://blog.fuzzynerd.com/~llansing/movers.pl.html -- another page which
exhibits this problem.  Didn't see the glitch until I added a "rowspan" to the
table.  Problem appears on the latest firefox and the latest mozilla.  Loading
from a local file works fine, loading from a remote server shows the problem.

I'll buy a case of beer to whoever manages to fix this one--it's annoying.

Comment 7

12 years ago
*** Bug 229303 has been marked as a duplicate of this bug. ***

Comment 8

12 years ago
*** Bug 301220 has been marked as a duplicate of this bug. ***

Comment 9

12 years ago
*** Bug 304189 has been marked as a duplicate of this bug. ***

Comment 10

12 years ago
(In reply to comment #1)
> To get the bug:
>     * The table must have border-collapse:collapse
>     * The table must have a table-cell with rowspan=4
The table may not have a table-cell with rowspan=4. Appears with any rowspan,
but less frequently.

Comment 11

12 years ago
When I switch CSS styles (to "No style" and back), the bug disappears. This
leads me to a simple conclusion that the bug appears when the DOM is loading;
when the style is applied to a DOM that's in it's final state, the bug doesn't
prove.

Comment 12

12 years ago
Created attachment 193476 [details]
Screenshot of the affected table border

Can be found at 
http://www.reznickepotreby.cz/?what=show;show=noze#table_be26b37cef8fa0bd23ed1f22ed416434

Comment 13

12 years ago
Workaround for the bug:

<style type="text/css">
  body.no_collapse table.border_collapse {
    border-collapse: separate;
    border-spacing: 0px;
    empty-cells: show;
  }
  body.no_collapse table.border_collapse td {
    border-bottom: 0px; border-right: 0px;
  }
</style>

<script type="text/javascript">
function onload(){
  AddClass(document.body, "no_collapse");
  RemClass(document.body, "no_collapse");
}
</script>


As I mentioned before, Gecko re-renders the table properly when the DOM is
completely loaded. So we can force it to re-render the tables by unsetting and
setting the border-collapse: collapse; . The CSS for <td> should make the
non-collapsed table look most similarly to the collapsed.

AddClass(...) and RemClass(...) may be found at
http://www.mauring.cz/lib.className_manager.js .

Comment 14

12 years ago
Created attachment 201169 [details]
table which has this problem in Firefox & Mozilla

This I page I wrote and posted online.  Firefox 1.0.7 (running on Windows XP) and Mozilal 1.7.3 (running on Linux) have trouble rendering some of the vertical borders when initially loading the page.  Interestingly, using "Print Preview" option (which inevitably looks garbled) and then closing the Print Preview window leaves the browser displaying the page correctly.

Comment 15

12 years ago
I also have this problem with rowspan+border-collapse. I worked around by setting border-collapse: seperate; and when the pages loads I set the border to collapse with javascript.

Comment 16

12 years ago
This bug even appears on one of the bugzilla pages:
Klick "Enter A Bug" and then search for any already reported bug. On my PC - using Win XP and Firefox 1.5 Branch - border lines never appear until i reload the iFrame.
One more page with the same problem:
http://www.athlon.de/ubbthreads.php?Cat=

Comment 17

12 years ago
(In reply to comment #16)
// using Firefox 1.8 Branch
would be nice to have this fixed for 1.5

Updated

12 years ago
Flags: blocking-aviary2?

Comment 18

12 years ago
(In response to comment #1 by Martijn Wargers and comment #10 by Ondra Zizka)

>> To get the bug:
>>     * The table must have border-collapse:collapse
>>     * The table must have a table-cell with rowspan=4
>The table may not have a table-cell with rowspan=4. Appears with any rowspan,
>but less frequently.

Having a rowspan=4 does not always trigger the bug. The two requirements above are necessary but not sufficient to trigger the bug. Random changes to cell contents, links etc may trigger or remove the bug...

The bug is still present in 1.5 and it shows a slightly different behaviour in Deer Park Alpha 2. When a page is loaded the borders shows correctly and then the horizontal and/or vertical borders might disappear and often no borders are visible for the "viewport", i.e. scrolling down reveals the borders for cells not visible cells. A reload can restore the horizontal and/or vertical borders. Small changes to the contents of the web page still effects the behaviour of the bug.
bumping nomination to Gecko list
Flags: blocking-firefox2? → blocking1.8.1?

Comment 20

11 years ago
I did not consistently notice the disappearing left and right borders in the attached test cases. http://www.dayah.com/periodic/disappearing-borders.php is too complex to be a test case, but shows the problem every time for me.

Comment 21

11 years ago
Any update on this bug?

Comment 22

11 years ago
I hate this bug too!
In my case the first page on my site will load OK, but when clicking on a link to another page which uses the same style sheet, as good as 99% of the time, the borders will not render properly.
Reloading the page fixes it always.
It seems my problem is table: collapse, and rowspan = 2.

Comment 23

11 years ago
Not going to block 1.8.1 for this.
Flags: blocking1.8.1? → blocking1.8.1-

Updated

11 years ago
Blocks: 365582

Updated

10 years ago
Summary: table borders disappear occasionally → table borders disappear occasionally with border-collapse

Updated

10 years ago
Summary: table borders disappear occasionally with border-collapse → table borders disappear occasionally with border-collapse and rowspan

Updated

10 years ago
Duplicate of this bug: 368201

Comment 25

10 years ago
The link mentioned in comment #20 has been moved to http://www.dayah.com/periodic/ still showing the same problem when hovering around the elements, particularly those on the bottom right.

Comment 26

10 years ago
This still happens in 2.0.0.4, but not in the most recent nightly.  

The number of rows in the table seems to influence this.  On a page with five tables, three with about 80-90 rows each and the other two much shorter, one of the large tables always seems to exhibit the problem but the shorter tables never seem to.  Here's the link to the specific page I'm talking about, in case it helps: http://gw.gamewikis.org/wiki/Necromancer_skills_quick_reference
I'm still seeing the bug with the most recent nightly with the first testcase in this bug.

Comment 28

10 years ago
My mistake, then.  I only tested the nightly against my own case, which was before I found this bug.

Updated

10 years ago
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-

Updated

10 years ago
Duplicate of this bug: 208542

Comment 30

10 years ago
One idea mentioned in some of these bugs is rounding/precision problems with thickness of borders. On my site showing the problem (private sorry), changing the border width from "thin" to "1px" or "2px" did NOT help - the borders still dissappear sometimes. Scrolling up and down partially redraws some borders. Will attach screenshot.

Comment 31

10 years ago
Created attachment 279079 [details]
Screenshot of partially rendered 2px borders

Updated

10 years ago
Duplicate of this bug: 351006

Updated

10 years ago
Duplicate of this bug: 267846

Comment 34

10 years ago
OS -> All, as users from other bugs say they see it on Linux
OS: Windows 2000 → All
Still shows on trunk builds. The most egregious problems that I've seen appear on http://en.wikipedia.org/wiki/Comparison_of_layout_engines_(CSS)

Problems there are not limited to the rowspanned cells.

The URL listed gives a 404 error; can anyone update it to include any of the other mentioned pages showing the error?

Comment 36

10 years ago
Of Bug 351006:
http://www.js-home.org/stellwerksim/zeitplan.php

See the bug for a screenshot.

Comment 37

10 years ago
Some factors on my page. It isn't random at all.

- Only affects internal, vertical borders.
- never drawn at top, but after a certain (fixed) row it is always drawn.
- however, due to painting different areas of the screen, the tops of the row where it starts working again may not be drawn, making it appear somewhat random.
- if, with partially drawn borders I resize, the whole window is repainted, and the vertical borders dissappear completely. 
- scrolling again will start them drawing.
- does not change with text size.

So it seems to be dependent on the row in the table that painting STARTS at. If painting starts after that row, it will be fine. If painting starts above that row, the borders never appear.

Examples:
1: After scrolling slowly downards, the borders after a certain row are drawn.
http://aaronlawrence.fastmail.fm/HalfDrawn.png
2: Resize, the borders dissappear
http://aaronlawrence.fastmail.fm/AfterResize.png
2: Scrolling down rapidly, partial borders appear (painting blocks different)
http://aaronlawrence.fastmail.fm/QuickScrollDown.png

Duplicate of this bug: 398694

Comment 39

10 years ago
However, the location down the page where the vertical borders dissappear is random depending on how long the page takes to load.
On a local file, they always work.
On a slow server load, they work from about halfway.
On a medium load, they work only on the bottom row.

I think it's fair to say that a slow connection (e.g. dialup) with lots of reflows probably makes it worse - no use trying this on your nice faster server with a broadband connection.

And sometimes the left border doesnt work either. Occasionally, horizontal borders dissappear. 

Trying to produce a simpler test case.

Comment 40

10 years ago
I give up. It seems to be timing and file size dependent, if you don't use a hack.

However, the following example shows the problem 99% most of the time for me, regardless of speed... (SM 1.0, FF 2.0.0.4)
http://aaronlawrence.fastmail.fm/VeryBad-random.htm
There are supposed to be black borders around all the cells but they usually appear only partially or not at all.

Anyway, I suppose this is all irrelevant, the first testcase shows the problem very easily and no developer has done anything about it for three years.
(Assignee)

Comment 41

10 years ago
Created attachment 284185 [details] [diff] [review]
Change nsTableFrame::CalcBCBorders to update borders it currently misses
Attachment #284185 - Flags: review?(bzbarsky)
(Assignee)

Comment 42

10 years ago
Created attachment 284187 [details]
Log of showing table with bug with DEBUG_TABLE_CELLMAP
(Assignee)

Comment 43

10 years ago
Created attachment 284188 [details]
Log of showing table without bug with DEBUG_TABLE_CELLMAP
(Assignee)

Updated

10 years ago
Attachment #284185 - Flags: review?(bzbarsky) → review?(bernd_mozilla)
(Assignee)

Comment 44

10 years ago
This patch is bound to be wrong, but hopefully bernd will point me in the right direction
(Assignee)

Updated

10 years ago
Attachment #284185 - Flags: review?(bernd_mozilla) → review?(fantasai.bugs)

Comment 45

10 years ago
Comment on attachment 284185 [details] [diff] [review]
Change nsTableFrame::CalcBCBorders to update borders it currently misses

Hi Malcolm, I think you got the wrong email address in that request. :)

Bernd, this is a one-line BC patch; Malcolm Parsons is asking you for review. If you don't have time, let me know and I'll give it a try, but I think you understand that part of the code a lot better than I do.
Attachment #284185 - Flags: review?(fantasai.bugs) → review?(bernd_mozilla)

Comment 46

10 years ago
Ah, finally some fresh air for this bug? Thanks :-)
IMHO, Mozilla should focus at fixing HTML problems like this and COLGROUP support etc. (I can't help myself writing it here, sorry for cluttering the bug's page.)

Also note that when the table is forced to redraw (by toggling border collapsing), it seems to be OK.

Comment 47

10 years ago
I got the first request already and to quote my mail:

<quote>

I am completely swamped with real life work, I reduced my mozilla work down to 0. I just watch the project. There are only a few who know border collapse code (fantasai, bz) well , but in general its a nightmare and to dive into it I would need around 6-8 hours of concentrated work to remember the stuff. I do not have these hours for a foreseeable future (4 weeks) and I would not even promise that this will be done after the 4 weeks. I am in this mode for over 5 months now.
So if you can wait then leave the request for me, if not ask fantasai.

Sorry for the bad news, but so you know that this will not be reviewed in time.

Bernd 
</quote>

I am not certain that this qualifies for approval.
(Assignee)

Updated

10 years ago
Attachment #284185 - Flags: review?(bernd_mozilla) → review?(fantasai.bugs)
(Assignee)

Comment 48

10 years ago
Created attachment 284827 [details] [diff] [review]
Fix *_DAMAGED macros

Looking at the log for the correctly rendered table, the required cellmap is:

***** START TABLE CELL MAP DUMP ***** 0x15f4730
cols array orig/span-> 0x15f47300=1/0 1=4/0 2=4/0 3=4/0  cols in cache 4

  ***** START GROUP CELL MAP DUMP ***** 0x15f0b60
  tbody mapRowCount=4 tableRowCount=4
  row 0 : C0,0  C0,1  C0,2  C0,3  
          t=101 t=101 t=101 t=101 
          l=101 l=141 l=141 l=141 
          c=110 c=120 c=120 c=120 
  row 1 : R     C1,1  C1,2  C1,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  row 2 : R     C2,1  C2,2  C2,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  row 3 : R     C3,1  C3,2  C3,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  C0,0=0x15f6a80(0)  C0,1=0x15f6e98(1)  C0,2=0x15f7058(2)  C0,3=0x15f7218(3)  
  C1,1=0x15f7460(1)  C1,2=0x15f7620(2)  C1,3=0x15f7840(3)  
  C2,1=0x15f7a88(1)  C2,2=0x15f7c48(2)  C2,3=0x15f7e08(3)  
  C3,1=0x15f8050(1)  C3,2=0x15f8210(2)  C3,3=0x15f83d0(3)  
  ***** END GROUP CELL MAP DUMP *****
***** bottom borders *****

          t=101 t=101 t=101 t=101 t=091 
          l=091 l=091 l=091 l=091 l=091 
          c=110 c=100 c=100 c=100 c=130 
***** END TABLE CELL MAP DUMP *****

Looking at the log for the incorrectly rendered table, we see that the final cell map is:

***** START TABLE CELL MAP DUMP ***** 0x153f0b0
cols array orig/span-> 0x153f0b00=1/0 1=4/0 2=4/0 3=4/0  cols in cache 4

  ***** START GROUP CELL MAP DUMP ***** 0x153f1f0
  tbody mapRowCount=4 tableRowCount=4
  row 0 : C0,0  C0,1  C0,2  C0,3  
          t=101 t=101 t=101 t=101 
          l=101 l=091 l=091 l=091 
          c=110 c=120 c=120 c=120 
  row 1 : R     C1,1  C1,2  C1,3  
          t=091 t=181 t=181 t=181 
          l=100 l=091 l=091 l=091 
          c=000 c=000 c=000 c=000 
  row 2 : R     C2,1  C2,2  C2,3  
          t=091 t=181 t=181 t=181 
          l=100 l=091 l=091 l=091 
          c=000 c=000 c=000 c=000 
  row 3 : R     C3,1  C3,2  C3,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  C0,0=0x1545c30(0)  C0,1=0x1546048(1)  C0,2=0x1546208(2)  C0,3=0x15463c8(3)  
  C1,1=0x1546610(1)  C1,2=0x15467d0(2)  C1,3=0x15469f0(3)  
  C2,1=0x1546c38(1)  C2,2=0x1546df8(2)  C2,3=0x1546fb8(3)  
  C3,1=0x154d6a0(1)  C3,2=0x154d860(2)  C3,3=0x154da20(3)  
  ***** END GROUP CELL MAP DUMP *****
***** bottom borders *****

          t=101 t=101 t=101 t=101 t=091 
          l=091 l=091 l=091 l=091 l=091 
          c=110 c=100 c=100 c=100 c=130 
***** END TABLE CELL MAP DUMP *****

Notice that the cells with missing borders have "l=091", where they should have "l=140" or "l=141".


Reading the rest of this log, we see that when the javascript was executed the cell map was built and looked like this:

***START TABLE DUMP*** 
mColWidths=7320 2160 2160 2160 
row(0)=0x1545920 cell(0)=0x1545c30 cell(1)=0x1546048 cell(2)=0x1546208 cell(3)=0x15463c8 
row(1)=0x1546588 cell(1)=0x1546610 cell(2)=0x15467d0 cell(3)=0x15469f0 
row(2)=0x1546bb0 cell(1)=0x1546c38 cell(2)=0x1546df8 cell(3)=0x1546fb8 
row(0)=0x154d618 cell(0)=0x154d6a0 cell(0)=0x154d860 cell(0)=0x154da20 
***** START TABLE CELL MAP DUMP ***** 0x153f0b0
cols array orig/span-> 0x153f0b00=1/0 1=3/0 2=3/0 3=3/0  cols in cache 4

  ***** START GROUP CELL MAP DUMP ***** 0x153f1f0
  tbody mapRowCount=4 tableRowCount=3
  row 0 : C0,0  C0,1  C0,2  C0,3  
          t=101 t=101 t=101 t=101 
          l=101 l=141 l=141 l=141 
          c=110 c=120 c=120 c=120 
  row 1 : R     C1,1  C1,2  C1,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  row 2 : R     C2,1  C2,2  C2,3  
          t=091 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=000 c=110 c=120 c=120 
  row 3 : R     
          t=101 
          l=091 
          c=110 
  C0,0=0x1545c30(0)  C0,1=0x1546048(1)  C0,2=0x1546208(2)  C0,3=0x15463c8(3)  
  C1,1=0x1546610(1)  C1,2=0x15467d0(2)  C1,3=0x15469f0(3)  
  C2,1=0x1546c38(1)  C2,2=0x1546df8(2)  C2,3=0x1546fb8(3)  
  
  ***** END GROUP CELL MAP DUMP *****
***** bottom borders *****

          t=091 t=101 t=101 t=101 t=091 
          l=091 l=091 l=091 l=091 l=091 
          c=000 c=100 c=100 c=100 c=130 
***** END TABLE CELL MAP DUMP *****

The cell map was correct at this point, so what happened next?

The log shows that nsTableFrame::InsertRows() is called, which then calls nsCellMap::InsertRows() which recides it needs to rebuild the table cell by cell because there is a rowspan in the damage area.

After the rebuild, all the cells have "l=091".

nsTableFrame::CalcBCBorders() is where the cells should get set to "l=140".

So why isn't it doing so in this case?

CalcBCBorders() looks at the damage area to decide which borders need to be recalculated.

In a debugger I see that in this case the borders are not in the damage area, and are not updated.

My initial patch changes CalcBCBorders() to partly ignore the damage area, so that the borders do get updated.

This works, but is inefficient.

So the question is, why are the borders not in the damage area?

CalcBCBorders() calls nsTableFrame::ExpandBCDamageArea() to make the damage area
slightly larger and if there are any rowspans in the damage area make the damage area the whole table.

Looking at this in a debugger, ExpandBCDamageArea() has indeed changed the local damgeArea variable to be the whole table.  So why hasn't it worked?

The damage area is tested using the TOP_DAMAGED, RIGHT_DAMAGED, BOTTOM_DAMAGED and LEFT_DAMAGED macros.

These macros look at the original damage area, not the expanded damage area  created by ExpandBCDamageArea()

So this new patch changes these macros to look at the local damage area variable.

These macros are not used in any other function, so this is safe.

I think this patch is obviously correct.
Attachment #284185 - Attachment is obsolete: true
Attachment #284827 - Flags: superreview?(bernd_mozilla)
Attachment #284827 - Flags: review?(fantasai.bugs)
Attachment #284185 - Flags: review?(fantasai.bugs)

Comment 49

10 years ago
Comment on attachment 284827 [details] [diff] [review]
Fix *_DAMAGED macros

I'm not a superreviewer. Please ask bz once you have a r+ from fantasai.
Attachment #284827 - Flags: superreview?(bernd_mozilla)

Comment 50

10 years ago
On the other hand comment 48 looks very reasonable and convincing to me. However I think one should remove the macros at all and replace the 2 callers of every macro with the explicit comparisons. Having a macro whcih takes a hidden local variable seems pretty evil to me. I believe the patch is correct, now it comes to coding style and this is where bz makes the final decisions. 
(Assignee)

Comment 51

10 years ago
Created attachment 284836 [details] [diff] [review]
Change the *_DAMAGED macros to take a second argument of the damage area to check

Same fix as previous patch, but with less evil macros.
Attachment #284827 - Attachment is obsolete: true
Attachment #284836 - Flags: review?(fantasai.bugs)
Attachment #284827 - Flags: review?(fantasai.bugs)

Comment 52

10 years ago
I believe

cellEndColIndex < damageArea.XMost()

is more compact than

RIGHT_DAMAGED(cellEndColIndex, damageArea)

and I do not see the benefit of the macro at all giving that there are only two places where it is called.
(Assignee)

Comment 53

10 years ago
Created attachment 284870 [details] [diff] [review]
Remove *_DAMAGED macros

Same fix, with the macros removed.

reftest added.
Attachment #284836 - Attachment is obsolete: true
Attachment #284870 - Flags: review?(fantasai.bugs)
Attachment #284836 - Flags: review?(fantasai.bugs)

Updated

10 years ago
Attachment #284870 - Flags: review?(fantasai.bugs) → review+
(Assignee)

Updated

10 years ago
Attachment #284870 - Flags: superreview?(bzbarsky)
Comment on attachment 284870 [details] [diff] [review]
Remove *_DAMAGED macros

>Index: layout/tables/nsTableFrame.cpp
>@@ -5341,17 +5336,17 @@ nsTableFrame::CalcBCBorders()
>+        if ((cellEndColIndex < damageArea.XMost()) && (rowX >= damageArea.y) && (rowX < damageArea.YMost())) {

This is overparenthesized.  Take out the extra parens, please.

Same thing in the other conditions you changed.

With that, sr=bzbarsky
Attachment #284870 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 55

10 years ago
Created attachment 284879 [details] [diff] [review]
Remove some brackets
Attachment #284870 - Attachment is obsolete: true

Updated

10 years ago
Assignee: nobody → malcolm.parsons

Updated

10 years ago
Attachment #284879 - Flags: approval1.9?
Attachment #284879 - Flags: approval1.9? → approval1.9+
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a9pre) Gecko/2007101504 Minefield/3.0a9pre ID:2007101504

What effect was this bug meant to have on attachment 201169 [details]? When I watch this testcase load the borders occasionally disappear during reflow, usually returning by the time the page has stopped loading, but not always. Is this problem a different bug?

Comment 57

10 years ago
Fix checked in last night. Many thanks to Bernd for monitoring this bug and reviewing Malcolm's patch, to roc for approving retroactively because I was dumb and forgot, and of course to Malcolm Parsons for figuring out the problem and solving it.

Marking fixed; stevee's problem seems to be a different bug.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(In reply to comment #57)
> Marking fixed; stevee's problem seems to be a different bug.

So please file a new bug on that, Steve.

Comment 59

10 years ago
He's added some comments to bug 318704.
(Assignee)

Comment 60

10 years ago
Created attachment 284972 [details]
New testcase

Still have a missing border bug when the reflow occurs in the last cell of the first row of a rowspan.
(Assignee)

Updated

10 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 61

10 years ago
Created attachment 284979 [details]
Log of showing table in attachment 284972 [details]

Correct when javascript triggers reflow:

  row 0 : C0,0  C0,1  C0,2  C0,3  
          t=101 t=101 t=101 t=101 
          l=101 l=141 l=141 l=141 
          c=110 c=120 c=120 c=120 
  row 1 : C1,0  C1,1  C1,2  C1,3  
          t=181 t=181 t=181 t=181 
          l=100 l=140 l=140 l=140 
          c=110 c=120 c=120 c=120 

...

Wrong after rebuild:

  row 0 : C0,0  C0,1  C0,2  C0,3  
          t=091 t=091 t=091 t=091 
          l=091 l=091 l=091 l=091 
          c=000 c=000 c=000 c=000 
  row 1 : C1,0  C1,1  C1,2  C1,3  
          t=091 t=091 t=091 t=091 
          l=101 l=141 l=141 l=141 
          c=020 c=020 c=020 c=020
(Assignee)

Comment 62

10 years ago
In this case, the cell map decides to rebuild, but the table frame decides that there are no rowspans overlapping the damage area.

This code is very fragile, with two bits having to make the same decision.

Why doesn't the cell map return the area that was damaged by the rebuild?
(Assignee)

Comment 63

10 years ago
Created attachment 284999 [details] [diff] [review]
Patch to fix the damage area returned from nsTableCellMap::InsertRows().  Reftest included

nsCellMap::InsertRows() returns the correct damage area, but nsTableCellMap::InsertRows() modifies it.

I think nsTableCellMap::InsertRows() is trying to make the damage area bigger,
so this patch stops it from making the damage area smaller.
Attachment #284999 - Flags: superreview?(bzbarsky)
Attachment #284999 - Flags: review?(fantasai.bugs)
(Assignee)

Comment 64

10 years ago
Attachment 201169 [details] renders correctly with this patch.
Comment on attachment 284999 [details] [diff] [review]
Patch to fix the damage area returned from nsTableCellMap::InsertRows().  Reftest included

I think I can happily r+sr this.  :)
Attachment #284999 - Flags: superreview?(bzbarsky)
Attachment #284999 - Flags: superreview+
Attachment #284999 - Flags: review?(fantasai.bugs)
Attachment #284999 - Flags: review+
Attachment #284999 - Flags: approval1.9?
Attachment #284999 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: ["Patch to fix the damage area" needs checkin]

Comment 66

10 years ago
Malcolm,

oh man, what a relief. I can't express how thankful I am for having you so naturally using these debug capabilities and finding the right answers.

Bernd
(Assignee)

Comment 67

10 years ago
I'd like to thank Martijn Wargers for creating the small testcase using javascript, as the debug output is unusable on the large testcases normally required to reproduce this bug.

And thanks to bz for helping me get a working build and pointing me at a function in the right area.  And to fantasai for help on IRC.

Larry: Don't worry about the beer.

Comment 68

10 years ago
Fix checked in, nsCellMap.cpp v 3.140. Thanks Malcolm!
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Keywords: checkin-needed

Updated

10 years ago
Attachment #284879 - Flags: approval1.8.1.9?

Updated

10 years ago
Attachment #284999 - Flags: approval1.8.1.9?
Comment on attachment 284879 [details] [diff] [review]
Remove some brackets

No comments saying how this fits the branch release criteria, appears to be out of scope
Attachment #284879 - Flags: approval1.8.1.10? → approval1.8.1.10-
Attachment #284999 - Flags: approval1.8.1.10? → approval1.8.1.10-
Blocks: 455093
No longer blocks: 455093

Updated

5 years ago
Depends on: 755519

Comment 70

3 years ago
Created attachment 8411115 [details]
Another new test case

This bug is marked as RESOLVED FIXED, but it seems to be back. I'm uploading a document containing a table that is affected by this bug. Tested on Firefox 29 (and Pale Moon 24.4.2).

Comment 71

3 years ago
frsiglmu, please file a new bug report and mention the new bug number here.
Flags: needinfo?(frsiglmu)

Comment 72

3 years ago
Got it.
The new report is Bug 1000435
Flags: needinfo?(frsiglmu)
You need to log in before you can comment on or make changes to this bug.