Closed
Bug 339246
Opened 19 years ago
Closed 18 years ago
Crash [@ nsIFrame::GetStyleData] called by nsTableRowFrame::CollapseRowIfNecessary
Categories
(Core :: Layout: Tables, defect)
Core
Layout: Tables
Tracking
()
RESOLVED
FIXED
People
(Reporter: MatsPalmgren_bugz, Assigned: bernd_mozilla)
References
Details
(4 keywords, Whiteboard: [sg:critical?] post-1.8 regression? retest)
Crash Data
Attachments
(3 files, 4 obsolete files)
18.89 KB,
text/plain
|
Details | |
484 bytes,
text/html
|
Details | |
17.37 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
Comment 2•19 years ago
|
||
Comment 3•19 years ago
|
||
Before the crash, I see: ###!!! ASSERTION: cell map has too many cols: 'numColsInCache == numColsInMap', file /Users/admin/trunk/mozilla/layout/tables/nsTableFrame.cpp, line 1055 The crash looks like a null deref: 0x0734cee4 in nsIFrame::GetStyleData (this=0x0, aSID=eStyleStruct_Visibility) at /Users/admin/trunk/mozilla/layout/base/../generic/nsIFrame.h:590 I did all my testing on Mac, so -> All/All.
OS: Linux → All
Hardware: PC → All
row 0 : C0,0 C0,1 C row 1 : C1,0 R R C C0,0=033B31E4(0) C0,1=033B3454(1) C1,0=033B35D4(0) ***** END GROUP CELL MAP DUMP ***** **** END TABLE CELL MAP DUMP ***** ***END TABLE DUMP*** ppended cell=033B3454 row=0 **** START TABLE CELL MAP DUMP ***** 03355200 ols array orig/span-> 033552000=3/0 1=1/2 2=0/2 cols in cache 3 ***** START GROUP CELL MAP DUMP ***** 033B40E0 tbody mapRowCount=2 tableRowCount=1 row 0 : C0,0 C C row 1 : R R C R C C0,0=033B3454(0) ***** END GROUP CELL MAP DUMP ***** **** END TABLE CELL MAP DUMP ***** ppended cell=033B35D4 row=1 **** START TABLE CELL MAP DUMP ***** 03355200 ols array orig/span-> 033552000=3/0 1=1/2 2=0/2 3=1/0 cols in cache 3 ***** START GROUP CELL MAP DUMP ***** 033B40E0 tbody mapRowCount=2 tableRowCount=2 row 0 : C0,0 C C row 1 : R R C R C C1,3 C0,0=033B3454(0) C1,3=033B35D4(3) ***** END GROUP CELL MAP DUMP ***** **** END TABLE CELL MAP DUMP ***** fter RemoveCell **** START TABLE CELL MAP DUMP ***** 03355200 ols array orig/span-> 033552000=3/0 1=1/2 2=0/2 3=1/0 cols in cache 3 ***** START GROUP CELL MAP DUMP ***** 033B40E0 tbody mapRowCount=2 tableRowCount=2 row 0 : C0,0 C C row 1 : R R C R C C1,3 <<<<<< the cell is in the fourth column while it should be in the third at least ideally even the second C0,0=033B3454(0) C1,3=033B35D4(3)
http://lxr.mozilla.org/seamonkey/source/layout/tables/nsCellMap.cpp#1989 is basically a MemMove operation where the overlap between dest and src becomes especially critical if zero span can indefinetely expand towards the end of the src.
The core issue is that nsCellMap::RebuildConsideringCells the most harsh form of cellmap adjustment, does not reset the column info. So when we rebuild the cellmap we use the old column information like the number of columns and since we add new data to the number of cells originating in a given column after one call to RebuildConsideringCells is always wrong. The attached patch will fix this, however my stomach tells me that this code is wrong as http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/tables/nsCellMap.cpp&rev=3.102&mark=1911-1913ݥ The reason to believe this is that the column info is shared between the rowgroup cellmaps and sits at the whole tablecellmap. So my guess is that one needs to rebuild the table cellmap rather than one group map.
this will reset the whole cellmap not only the group one, by this we are allowed to reset the colinfo
Attachment #225203 -
Attachment is obsolete: true
Assignee | ||
Comment 10•19 years ago
|
||
Attachment #225471 -
Attachment is obsolete: true
Comment 11•19 years ago
|
||
The newest patch touches a different file than the previous patch. Does the newest patch replace the previous patch or complement it?
Assignee | ||
Comment 12•19 years ago
|
||
that was the wrong patch
Attachment #225587 -
Attachment is obsolete: true
Assignee | ||
Comment 13•18 years ago
|
||
Attachment #225604 -
Attachment is obsolete: true
Attachment #226961 -
Flags: superreview?(bzbarsky)
Attachment #226961 -
Flags: review?(bzbarsky)
![]() |
||
Updated•18 years ago
|
Attachment #226961 -
Flags: superreview?(bzbarsky)
Attachment #226961 -
Flags: superreview+
Attachment #226961 -
Flags: review?(bzbarsky)
Attachment #226961 -
Flags: review+
Assignee | ||
Comment 14•18 years ago
|
||
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 15•18 years ago
|
||
This reduced testcase does not crash Bon Echo or ff1.5.0.4 (winXP). Is it a regression? If so from what? If we land the other stir-table fixes will we need this one on the branch anyway? This testcase looks like a null-deref as Jesse noted, but the description in comment 6 makes it sound potentially worse. "sg:critical?" then?
Whiteboard: [sg:critical?] post-1.8 regression? retest
Comment 16•18 years ago
|
||
Probably don't need on branches, but nominating until we get a definitive answer.
Flags: blocking1.8.0.6?
Updated•18 years ago
|
Flags: blocking1.8.0.6?
Comment 17•18 years ago
|
||
As Bernd said in bug 341814, comment 6, it fixes the crash in bug 341814, which can be seen on branch. The testcase in bug 341814 crashes branch builds.
Assignee | ||
Comment 18•18 years ago
|
||
fixed on branches by the cellmap branch patch
Keywords: fixed1.8.0.7,
fixed1.8.1
Comment 19•18 years ago
|
||
v.fixed on both branches, no crash with testcase from bug 341814 (did crash 1.5.0.6): Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7) Gecko/20060829 Firefox/1.5.0.7 Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060830 BonEcho/2.0b2 NOTE: Jesse's reduced testcase in this bug did not crash any of my branch builds (1506, 1507, or bonecho)
Updated•17 years ago
|
Group: security
Flags: in-testsuite?
Comment 20•16 years ago
|
||
crash test landed http://hg.mozilla.org/mozilla-central/rev/2fae448c1311
Flags: in-testsuite? → in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ nsIFrame::GetStyleData]
You need to log in
before you can comment on or make changes to this bug.
Comment 1
•