54124 - assertions in table cell code

Status: RESOLVED INVALID

(Core :: Layout, defect, P2)

Description

[David :Bienvenu]

If you go to the above url, you'll assert like crazy and eventually crash. I'll
attach a patch that fixes the crash so you can see where the crash is.
mBorderEdges is null; I don't know why. I had to comment out some assertions in
the patch so that I could see if the patch fixes the crash, so you don't want to
check the patch in as is.

Comment 1

[David :Bienvenu]

Attachment: patch that fixes the problem

Comment 2

[David :Bienvenu]

You'll need to update in layout to get the fix for 52722 to even get to see this
bug with the above url; otherwise you'll crash before. Please let me know if you
have any trouble reproducing this.

Comment 3

[karnaze (gone)]

Marking nsbeta3+.

Comment 4

[karnaze (gone)]

Marking nsbeta3-, rtm+.

Comment 5

[David :Bienvenu]

marking p2; otherwise pdt will never see it.

Comment 6

[Phil Peterson]

PDT marking [rtm need info] until code reviews are available.

Comment 7

[David :Bienvenu]

Chris, I'm assuming you're taking care of this - you can put me down as one of
the reviewers to save some of your time but you'll need a super reviewer.

Comment 8

[karnaze (gone)]

David, thanks, I'll take care of this. If 41262 gets fixed in the next few days 
then this bug will not be relevant. If 41262 doesn't get fixed then 49490 may 
have to be fixed and this bug is still not relevant. However, if this bug is 
seriously impacting things before then, I could get it in.

Comment 9

[David :Bienvenu]

OK. FYI, 41262 is not nominated for RTM, so it seems unlikely it will get fixed.
 And 49490 is nominated, but doesn't have an rtm+, so presumably no work is
being done on it.

Comment 10

[karnaze (gone)]

Attachment: revised patch with more null checks

Comment 11

[karnaze (gone)]

I'm marking this rtm- and dependent on bug 49490 which must get fixed. After 
applying the patch and commenting out a bunch of other assertions in table code 
and line layout, it crashes in nsCSSRendering::DrawDashedSegments.

Comment 12

[karnaze (gone)]

Changing "crash" in summary to "assertions", removing crash keyword and adding 
qawanted. The patches are not needed since bug 49490 was checked in. There are 
still a few assertions in table code, but it doesn't appear to be causing a 
problem. I'm leaving this open due to the assertions. A smaller test case is 
needed.

Comment 13

[Bernd]

chris wrote:

A smaller test case is needed.

http://client/gecko/embed/EmbedGecko.html

is netscape internal, in oder to get external qa the page should be visible to
the outsider's.

bernd

Comment 14

[karnaze (gone)]

Marking invalid because the code has been disabled and will be replaced with the 
new collapsing border code.