Closed Bug 348049 Opened 18 years ago Closed 18 years ago

[FIX]Frames not destroyed with this XBL + <td> testcase

Categories

(Core :: XBL, defect, P1)

Product:
Core
Component:
XBL
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9alpha1

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(4 keywords, Whiteboard: [sg:critical])

Attachments

(2 files)

testcase (non-crashing)
(deleted), application/xhtml+xml
Details
I think this is the right fix
(deleted), patch
roc
: review+
roc
: superreview+
dveditz
: approval1.8.0.7+
dbaron
: approval1.8.1+
Details | Diff | Splinter Review
1. Load either testcase. Result: ###!!! ASSERTION: unsupported operation: 'PR_FALSE', nsTableCellFrame::AppendFrames 2. Reload. Result: Assuming the patch from 334514 is applied, you'll see another assertion: "Some frame destructors were not called". Result: If you're using the crashing testcase, you're likely to see a crash attempting to dereference 0xdadadaf6.
Whiteboard: [sg:critical]
Attached file testcase (non-crashing) (deleted) —
Attached file (deleted) —
Blocks: 348483
Attached patch I think this is the right fix (deleted) — — Splinter Review
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #233506 - Flags: superreview?(roc)
Attachment #233506 - Flags: review?(roc)
roc, what do you think of the branch-safety of this patch? I think it's pretty safe myself....
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: Macintosh → All
Summary: Frames not destroyed with this XBL + <td> testcase → [FIX]Frames not destroyed with this XBL + <td> testcase
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 233506 [details] [diff] [review] I think this is the right fix Well, it does change behaviour in several cases, but everywhere it changes behaviour it's almost certainly fixing a similar bug, so I think we should take it on branches.
Attachment #233506 - Flags: superreview?(roc)
Attachment #233506 - Flags: superreview+
Attachment #233506 - Flags: review?(roc)
Attachment #233506 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 233506 [details] [diff] [review] I think this is the right fix We should probably take this on branches. This doesn't change behavior in most cases; all the ones where it does probably led to crashes anyway.
Attachment #233506 - Flags: approval1.8.1?
Attachment #233506 - Flags: approval1.8.0.7?
Comment on attachment 233506 [details] [diff] [review] I think this is the right fix a=dbaron on behalf of drivers. Please check in to the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword once you have done so.
Attachment #233506 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 233506 [details] [diff] [review] I think this is the right fix approved for 1.8.0 branch, a=dveditz for drivers
Attachment #233506 - Flags: approval1.8.0.7? → approval1.8.0.7+
Fixed on branches.
Keywords: fixed1.8.0.7, fixed1.8.1
https://bugzilla.mozilla.org/attachment.cgi?id=232924 ff2b2 debug windows/linux no assert https://bugzilla.mozilla.org/attachment.cgi?id=232925 ff2b2 debug/nightly windows/linux no crash verified fixed 1.8 (happyfish)
Keywords: fixed1.8.1 → verified1.8.1
https://bugzilla.mozilla.org/attachment.cgi?id=232925&action=view should not crash browser Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060821 Firefox/1.5.0.7pre verified 1.8.0.7 more happy fishes!
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.7 → verified1.8.0.7
Blocks: 323926
Group: security
Flags: in-testsuite?
Attachment #232925 - Attachment is private: true
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
I checked in a reftest for this too.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: