308752 - table crash [@ BCMapCellIterator::SetInfo] [@ nsTableFrame::GetEffectiveColSpan]

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Comment 1

[Jesse Ruderman]

Attachment: testcase (not reduced)

Comment 2

[Bernd Mielke]

taking, 

Can I add Martin Wargers to security related bugs, he is a testcase createur
extraordinaire

Comment 3

[Jesse Ruderman]

Yes.

Comment 4

[Martijn Wargers (dead)]

Attachment: Testcase2 (not reduced)

I managed to reduce it a bit further, but it is still not minimal.
When you get a slow script warning, just continue.
After the page has finished consuming all cpu, click once in the page to get
the crash.
The style table  { border-collapse: collapse;  } seems also necessary to
trigger the crash.

Comment 5

[Martijn Wargers (dead)]

Attachment: source of parentNode of allNodes[60]

This is how the dom source looks like of the parentNode of allNodes[60] just
before the crash. allNodes[60] is the second td.

Comment 6

[Bernd]

I see the crash with attachment 196333 [details] as described in comment 4

Comment 7

[Bernd]

I tried today to get this testcase down to something reasonable, but failed. Is
there any known way how to rewind these random things into something usefull?
With the attached testcase its impossible for me to debug this.

Comment 10

[Bernd]

Martijn, that testcase is great, its reasonable small, crashes and is  debugable.

Comment 11

[Bernd]

Attachment: slightly shorter testcase

Comment 12

[Bernd]

Attachment: patch

Comment 13

[Jesse Ruderman]

Bernd, is this patch ready for review?

Comment 14

[Bernd]

I need to rtest the patch and to add some comments at least to the bug or better
to the source to get this into a shape where I can ask Boris to review this.

Comment 15

[Bernd]

Attachment: patch

The problem here is that rebuild cellmap relied on Appendcells to increase the
mRowCount. This fails if an empty row is in the table, as AppendCells will not
be called in this case. Only  when the rows are to be added or to be removed we
know for certain that they are real and not dead rows so putting the burden
here at AppendCells is wrong.

Comment 16

[Bernd]

fix checked in

Comment 17

[Mike Schroepfer]

Comment on attachment 197277 [details] [diff] [review]
patch

Did this land on trunk or branch?

Comment 18

[Boris Zbarsky [:bzbarsky]]

This has so far landed on trunk but not branch.

Comment 19

[Bernd]

I am not sure that I will be able to checkin before friday, if this is urgent
then please check it in before.

Comment 20

[Bernd]

fixed on branch

Comment 21

[Marcia Knous [:marcia]]

Using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b5)
Gecko/20050928 Firefox/1.4, I still crash using the first test case, as well as
the fourth test case. Reopening. Talkback ID is TB9840133M for the first
testcase. Running OSX 10.4.2.

Comment 22

[Marcia Knous [:marcia]]

resolving fixed again - I will test tomorrow. Misread the checkin date. sorry.

Comment 23

[Marcia Knous [:marcia]]

Looks good using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
rv:1.8b5) Gecko/20050929 Firefox/1.4. No crashes with any of the test cases. If
I run the first test case, Firefox does throw up an alert "Unresponsive script."
Is this expected?

Comment 25

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/6336a2d9d0d9