302118 - issues with extensive HTML tables in combination with CSS [@ ntdll.dll]

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Age Bosma]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

When having a huge HTML table with many rows and columns Firefox starts having
rendering issues as soon as you start using CSS to style the table. The issues
appear to be random:
- Somtimes it displays the table correctly.
- Sometimes it only renders the table incorrectly with e.g. the vertical borders
being almost collapsed all the way to the left.
- Sometimes it even manages to crash Firefox completely.

Reproducible: Sometimes

Steps to Reproduce:
1. Open the provided attachment in Firefox
2. When it renders the tables correctly at first, try to refresh the page a
couple of times.



The problems appear to occur more often when colgroups are being used with a
<col width="" /> for each column.

Comment 1

[Age Bosma]

Attachment: Table rendering test case

Comment 2

[Age Bosma]

This bug might be related to bug 282747

Comment 3

[Age Bosma]

Attachment: Table render 1

When colgroup's and col's are included this is what usually happens (if it
doesn't crash the browser)

Comment 4

[Age Bosma]

Attachment: Table render 2

When colgroup's and col's are not included this is what usually happens.

Comment 5

[Maarten Brouwers]

Crash confirmed on 'Table rendering test case' (text/html) using Mozilla/5.0
(Windows; U; Windows NT 5.1; en-GB; rv:1.7.8) Gecko/20050511 Firefox/1.0.4 

Machine runs Windows XP SP2

Note that this page is not validating XHTML1.1, still this should not make
firefox crash (which happened here). 

Comment 6

[O. Atsushi (Torisugari)]

Crashed with attachment 190484 [details]

TB7814273M

Comment 7

[Age Bosma]

Attachment: Table rendering testcase (valid HTML)

Here's a HTML valid testcase. Firefox appears to alter the HTML source when
saving a HTML file by stripping it from trailing slashes :-S

Comment 8

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050726
Firefox/1.0+ ID:2005072610
I also crashed on 'Table rendering test case' attachment.
TB7830011Y

Comment 9

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050726
Firefox/1.0+ ID:2005072610

No crash and renders fine/perfect .. 

Comment 10

[Age Bosma]

(In reply to comment #9)
> Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050726
> Firefox/1.0+ ID:2005072610
> 
> No crash and renders fine/perfect .. 

Try refreshing the page a couple of times more, the problems do occur.

Comment 11

[Peter van der Woude [:Peter6]]

(In reply to comment #10)
> Try refreshing the page a couple of times more, the problems do occur.

I tried 
F5 10 times
ctrl+F5 10 times
reload button 10 times

no crash, no rendering problems

Comment 12

[O. Atsushi (Torisugari)]

> [@ ntdll.dll]

I have another talkback id (caused by
attachment 190531 [details] )

TB7814961M

Now I can reproduce this crash via
wheel scroll during loading.

Comment 13

[Peter van der Woude [:Peter6]]

(In reply to comment #12)
> > [@ ntdll.dll]
> 
> I have another talkback id (caused by
> attachment 190531 [details] [edit] )
> 
> TB7814961M
> 
> Now I can reproduce this crash via
> wheel scroll during loading.

Scrolling doesn't crash me either.
This may well be fixed on Trunk.
Can anyone else verify with a latest nightly ?

Comment 14

[Age Bosma]

(In reply to comment #13)
> 
> Scrolling doesn't crash me either.
> This may well be fixed on Trunk.
> Can anyone else verify with a latest nightly ?
> 

Have it your way ;-)

I can verify that this bug DOES occur in the latest nightly:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050726
Firefox/1.0+

I installed the nightly downloaded on 2005/07/27 00:41:00 GMT. It only required
one page refresh to crash the browser.

Comment 15

[Age Bosma]

Any news on this bug? Shouldn't this bug be blocking the Firefox 1.5 release
since it's simply crashing the browser?

Comment 16

[Adam Guthrie]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050829
Firefox/1.0+

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050828
Firefox/1.6a1

Works for me.

Comment 17

[Gary Elshaw]

Crashed test case on Branch Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O;
en-US; rv:1.8b4) Gecko/20050829 Firefox/1.0+ with OS X 10.4.2 on second reload.
Talkback submitted.

Comment 18

[Martijn Wargers (dead)]

I get a 100% reproducable crash, when I visit the testcase and let the "clone
slowly" bookmarklet loose on it:
http://www.squarefree.com/bookmarklets/testbrowsers.html#clone_slowly
(at the bottom)

Talkback ID's: TB8844634Z TB8844623H TB8844610Z

Using a nightly trunk build. Will try tomorrow to get a minimal testcase.

Comment 19

[Bernd]

nsStyleBorder::~nsStyleBorder() line 333 + 3 bytes
nsStyleBorder::Destroy(nsPresContext * 0x04c23650) line 465 + 8 bytes
TableBackgroundPainter::TableBackgroundData::Destroy(nsPresContext * 0x04c23650)
line 149
TableBackgroundPainter::~TableBackgroundPainter() line 263
nsTableFrame::Paint(nsTableFrame * const 0x04c373bc, nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 1) line 1416
nsContainerFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c373bc, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 283
nsTableOuterFrame::Paint(nsTableOuterFrame * const 0x04c3720c, nsPresContext *
0x04c23650, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 335
nsContainerFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c3720c, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 283
nsBlockFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c3720c, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 287
PaintLine(const nsRect & {...}, const nsRect & {...}, nsLineList_iterator &
{...}, int 0, int & 1238536, nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, nsFramePaintLayer eFramePaintLayer_Underlay, nsBlockFrame * 0x04c4bf18)
line 6352
nsBlockFrame::PaintChildren(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay,
unsigned int 0) line 6420 + 38 bytes
nsHTMLContainerFrame::PaintDecorationsAndChildren(nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, int 1, unsigned int 0) line 138
nsBlockFrame::Paint(nsBlockFrame * const 0x04c4bf18, nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 6247
nsContainerFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c4bf18, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 283
nsBlockFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c4bf18, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 287
PaintLine(const nsRect & {...}, const nsRect & {...}, nsLineList_iterator &
{...}, int 0, int & 1239576, nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, nsFramePaintLayer eFramePaintLayer_Underlay, nsBlockFrame * 0x04c4bd94)
line 6352
nsBlockFrame::PaintChildren(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay,
unsigned int 0) line 6420 + 38 bytes
nsHTMLContainerFrame::PaintDecorationsAndChildren(nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, int 1, unsigned int 0) line 138
nsBlockFrame::Paint(nsBlockFrame * const 0x04c4bd94, nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 6247
nsContainerFrame::PaintChild(nsPresContext * 0x04c23650, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x04c4bd94, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 283
nsContainerFrame::PaintChildren(nsPresContext * 0x04c23650, nsIRenderingContext
& {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay,
unsigned int 0) line 228
nsHTMLContainerFrame::Paint(nsHTMLContainerFrame * const 0x04c3922c,
nsPresContext * 0x04c23650, nsIRenderingContext & {...}, const nsRect & {...},
nsFramePaintLayer eFramePaintLayer_Underlay, unsigned int 0) line 84
CanvasFrame::Paint(CanvasFrame * const 0x04c3922c, nsPresContext * 0x04c23650,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay, unsigned int 0) line 369 + 22 bytes
PresShell::Paint(PresShell * const 0x04c383a8, nsIView * 0x04c4cca8,
nsIRenderingContext & {...}, const nsRect & {...}) line 5652 + 27 bytes
nsView::Paint(nsView * const 0x04c4cca8, nsIRenderingContext & {...}, const
nsRect & {...}, unsigned int 0, int & 0) line 316
nsViewManager::RenderDisplayListElement(DisplayListElement2 * 0x04e20858,
nsIRenderingContext * 0x04e20c48) line 1468
nsViewManager::RenderViews(nsView * 0x04c48ad0, nsIRenderingContext & {...},
const nsRegion & {...}, nsIDrawingSurface * 0x04e11f50, const nsVoidArray &
{...}) line 1380
nsViewManager::Refresh(nsView * 0x04c48ad0, nsIRenderingContext * 0x04e20c48,
nsIRegion * 0x04e1b628, unsigned int 1) line 930
nsViewManager::DispatchEvent(nsViewManager * const 0x04c249d8, nsGUIEvent *
0x0012f434, nsEventStatus * 0x0012f2f8) line 2055
HandleEvent(nsGUIEvent * 0x0012f434) line 174
nsWindow::DispatchEvent(nsWindow * const 0x04c4cae4, nsGUIEvent * 0x0012f434,
nsEventStatus & nsEventStatus_eIgnore) line 1061 + 9 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f434, nsEventStatus &
nsEventStatus_eIgnore) line 1087
nsWindow::OnPaint(HDC__ * 0x00000000) line 5560 + 28 bytes
nsWindow::ProcessMessage(unsigned int 15, unsigned int 0, long 0, long *
0x0012f9ac) line 4169 + 19 bytes
nsWindow::WindowProc(HWND__ * 0x00020376, unsigned int 15, unsigned int 0, long
0) line 1243 + 24 bytes
USER32! 77d18734()
USER32! 77d18816()
USER32! 77d1b4c0()
USER32! 77d1b50c()
NTDLL! 7c91eae3()
USER32! 77d18a10()
nsAppShell::Run(nsAppShell * const 0x00fc85a8) line 135
nsAppStartup::Run(nsAppStartup * const 0x00fc8508) line 145 + 77 bytes
XRE_main(int 3, char * * 0x003c6eb8, const nsXREAppData * 0x00422020 kAppData)
line 2322 + 34 bytes
main(int 3, char * * 0x003c6eb8) line 61 + 16 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7

Comment 20

[Bernd]

this is 0xdddddddd

Comment 21

[Bernd]

Martijn, it might be hard to create a nice minimized crash testcase but a
testcase that triggers  ASSERTION: colgroup data should not be null - bug
237421: would help definetely.

Comment 22

[Martijn Wargers (dead)]

Attachment: Testcase

This is the most minimal testcase that I could come up with that still crashes.

It crashes when you try to select the text in the table.
It probably gives an assertion too (don't know yet).

Comment 23

[Adam Guthrie]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050830 Firefox/1.6a1
ID:2005083010

Slightly different crash on Linux with Martijn's testcase. This does give the
assersion mentioned above.

Incident ID: 8870271
Stack Signature	firefox-bin + 0x402 (0x00ba8402) 29c88386
Product ID	FirefoxTrunk
Build ID	2005082205
Trigger Time	2005-08-30 11:24:13.0
Platform	LinuxIntel
Operating System	Linux 2.6.12-1.1398_FC4
Module	firefox-bin + (00000402)
URL visited	
User Comments	
Since Last Crash	0 sec
Total Uptime	33 sec
Trigger Reason	SIGIOT: Abort or IOT Instruction: (signal 6)
Source File, Line No.	N/A
Stack Trace 	
firefox-bin + 0x402 (0x00ba8402)
libc.so.6 + 0x29fc8 (0x00eb1fc8)
libc.so.6 + 0x5d96a (0x00ee596a)
libc.so.6 + 0x63864 (0x00eeb864)
libc.so.6 + 0x63d9f (0x00eebd9f)
libstdc++.so.5 + 0x912b3 (0x007c12b3)
TableBackgroundPainter::PaintTable() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/tables/nsTablePainter.cpp,
line 453]
ProcessRowInserted() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/tables/nsTableFrame.cpp,
line 1636]
SyncFrameViewGeometryDependentProperties() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 497]
nsTableOuterFrame::GetCaptionAvailWidth() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/tables/nsTableOuterFrame.cpp,
line 51]
SyncFrameViewGeometryDependentProperties() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 497]
nsBlockFrame::HandleEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 1390]
CanvasFrame::QueryInterface() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsHTMLFrame.cpp,
line 198]
nsBlockFrame::HandleEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 6603]
SyncFrameViewGeometryDependentProperties() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 497]
nsBlockFrame::HandleEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 1390]
CanvasFrame::QueryInterface() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsHTMLFrame.cpp,
line 198]
nsBlockFrame::HandleEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 6603]
SyncFrameViewGeometryDependentProperties() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 497]
HasNonZeroBorderRadius() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 463]
NS_NewCanvasFrame() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsHTMLFrame.cpp,
line 175]
nsHTMLReflowState::nsHTMLReflowState() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp,
line 121]
PresShell::HandleEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/layout/base/nsPresShell.cpp,
line 202]
nsIView::CreateWidget() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsView.cpp,
line 235]
nsViewManager::UpdateWidgetArea() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsViewManager.cpp,
line 55]
nsViewManager::CreateBlendingBuffers() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsViewManager.cpp,
line 1554]
SortByZOrder() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsViewManager.cpp,
line 61]
nsViewManager::DispatchEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsViewManager.cpp,
line 56]
nsView::ResetWidgetBounds() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/view/src/nsView.cpp,
line 60]
nsBaseWidget::GetRenderingContext() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/widget/src/xpwidgets/nsBaseWidget.cpp,
line 1396]
nsWindow::OnScrollEvent() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp,
line 1836]
nsWindow::IMEComposeText() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp,
line 4440]
libgtk-x11-2.0.so.0 + 0x10b352 (0x0021f352)
libgobject-2.0.so.0 + 0x8bc8 (0x00a32bc8)
libgobject-2.0.so.0 + 0x185be (0x00a425be)
libgobject-2.0.so.0 + 0x19c55 (0x00a43c55)
libgobject-2.0.so.0 + 0x1a249 (0x00a44249)
libgtk-x11-2.0.so.0 + 0x1e6ac3 (0x002faac3)
libgtk-x11-2.0.so.0 + 0x109fa5 (0x0021dfa5)
libgdk-x11-2.0.so.0 + 0x295ad (0x004335ad)
libgdk-x11-2.0.so.0 + 0x29680 (0x00433680)
libgdk-x11-2.0.so.0 + 0x29701 (0x00433701)
libglib-2.0.so.0 + 0x272ee (0x0099e2ee)
libglib-2.0.so.0 + 0x2507e (0x0099c07e)
libglib-2.0.so.0 + 0x28096 (0x0099f096)
libglib-2.0.so.0 + 0x28383 (0x0099f383)
libgtk-x11-2.0.so.0 + 0x1091b5 (0x0021d1b5)
GdkKeyCodeToDOMKeyCode() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/widget/src/gtk2/nsGtkKeyUtils.cpp,
line 174]
nsDownloadManager::Observe() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,
line 503]
XRE_main() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/toolkit/xre/nsAppRunner.cpp,
line 2249]
CheckArg() 
[/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/toolkit/xre/nsAppRunner.cpp,
line 331]
libc.so.6 + 0x1549f (0x00e9d49f)

From a debug build: 
###!!! ASSERTION: colgroup data should not be null - bug 237421:
'mCols[i].mColGroup', file
/home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 340
Break: at file /home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 340
###!!! ASSERTION: colgroup data should not be null - bug 237421:
'mCols[i].mColGroup', file
/home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 340
Break: at file /home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 340
###!!! ASSERTION: colgroup data should not be null - bug 237421:
'mCols[i].mColGroup', file
/home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 259
Break: at file /home/aguthrie/moz/mozilla/layout/tables/nsTablePainter.cpp, line 259
*** glibc detected *** ./firefox-bin: double free or corruption (fasttop):
0x0a0ad4c0 ***
...

Comment 24

[Bernd]

the testcase demonstrates that the issues in bug 281241 not only lead to wrong
layout but to crashes. This is no surprise as wrong frame structures are nearly
a guarantee for crash bugs.

Comment 25

[Bernd]

Attachment: crash fix

Comment 26

[Bernd]

Comment on attachment 194667 [details] [diff] [review]
crash fix

Boris this is the thing that we talked on IRC last week. The usual suspects,
the table background painter relies on correct column information. Nobody did
that so strict before, so we need to fix the old  column bugs. But I think we
are pretty stable already now.

Comment 27

[Boris Zbarsky [:bzbarsky]]

I won't be able to look at this until Tuesday.

Comment 28

[Bernd]

np, I will change my flat, so I won't have time before friday to check it in.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 194667 [details] [diff] [review]
crash fix

>Index: nsTableColGroupFrame.cpp
> nsTableColGroupFrame::AppendFrames(nsIAtom*        aListName,
>+  nsTableColFrame* col = GetFirstColumn();
>+  nsTableColFrame* nextCol;
>+  while (col && col->GetColType() == eColAnonymousColGroup) {
>+	// this colgroup  spans one ore more columns but now that there is 
>+	// real column below, so  spanned anonymous columns should be removed

I'm not sure I follow what's going on here...  What does it mean for the type
of a col to be eColAnonymousColGroup?  That it's been created because the
colgroup didn't have enough child cols and had a span, I guess?

>+    nextCol = col->GetNextCol();
>+    RemoveFrame(nsnull, col);

So we're just removing all such synthetic col frames that we inserted?	Would
those always be at the front of the frame list?

>+	col = nextCol;

Fix indent, please.  Same in a few other places.

Could you document the various functions that take col indices in
nsTableColGroupFrame.h with a brief explanation of what the indices there
should be?

Comment 30

[Bernd]

>That it's been created because the
>colgroup didn't have enough child cols and had a span, I guess?

Not exactly take a single <colgroup> tag this will create one anonymous col below.
A <colgroup span="5"> will the create 5 anonymous cols below.
<colgroup span="5"><col></colgroup> will create 1!! content based col below and
ignore the span="5".
so what happens here is that we toggle from

<colgroup span="1"/>

to

<colgroup>
 <col>
</colgroup>

A colgroup can have only anonymous cols below or content based cols, it should
never mix those two types.

If there are content based cols then one can have span on these cols too. so one
would get eColAnonymousCol as a type for the spanned cols.

The last but most fancy type are the eColAnonymousCell type columns. Each table
even if no cols are specified creates col frames to cover all columns in the
table. You can have more columns than one would derive from the cellmap but
*never* less. If there are less columns then this last type is created. If it is
created a special anonymous colgroup is also created which harbors these
anonymous cell based columns.

Comment 31

[Bernd]

whats wrong with the comments that I already added
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&file=nsTableColGroupFrame.h&rev2=3.59&rev1=3.58

Comment 32

[Bernd]

or do you mean the valid range of the arguments?

Comment 33

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 194667 [details] [diff] [review]
crash fix

I don't know how I missed those comments... :(

>Index: nsTableColGroupFrame.cpp

> nsTableColGroupFrame::AppendFrames(nsIAtom*        aListName,
>+  while (col && col->GetColType() == eColAnonymousColGroup) {
>+	// this colgroup  spans one ore more columns but now that there is 
>+	// real column below, so  spanned anonymous columns should be removed

Fix indent.  And "colgroup spans one or more" (not "ore", and remove extra
space after "colgroup".  Also, remove the "so " after the comma.

>+    RemoveFrame(nsnull, col);
>+	col = nextCol;

Fix indent.

> nsTableColGroupFrame::InsertFrames(nsIAtom*        aListName,

Same comments here.

>@@ -625,12 +645,21 @@ void nsTableColGroupFrame::Dump(PRInt32 
>+  NS_ASSERTION((j- GetStartColumnIndex()) == GetColCount(), 

Space after |j|, please.

r+sr=bzbarsky

Comment 34

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch updated to my comments

Comment 35

[Age Bosma]

Is this bug fix already included in the nightly's? I would like to test if it
fixes the crash and redering problems.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Fix checked in on trunk.  Should be in tomorrow's nightlies.

Comment 37

[Bernd]

thanks Boris, you da man. Did I mention how dialin sucks?

Comment 38

[Age Bosma]

Brilliant! It works like a charm, no more crashes, no more rendering issues.
Thank you very much!

Comment 39

[Martijn Wargers (dead)]

Verified. Maybe this is something for the branch? Fixes a crash.

Comment 40

[Asa Dotzler [:asa]]

Comment on attachment 194667 [details] [diff] [review]
crash fix

please do land this on the branch if it's safe.

Comment 41

[Boris Zbarsky [:bzbarsky]]

Fixed on branch

Comment 42

[Jay Patel [:jay]]

v.fixed on branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5)
Gecko/20050928 Firefox/1.4, no crashes and rendering is correct.