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

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
XBL
P1
critical
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9alpha1
crash, testcase, verified1.8.0.7, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Updated

11 years ago
Whiteboard: [sg:critical]
(Reporter)

Comment 1

11 years ago
Created attachment 232924 [details]
testcase (non-crashing)
(Reporter)

Comment 2

11 years ago
Created attachment 232925 [details]
testcase (crashing)
(Reporter)

Updated

11 years ago
Blocks: 348483
Created attachment 233506 [details] [diff] [review]
I think this is the right fix
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
Last Resolved: 11 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

Comment 11

11 years ago
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
(Assignee)

Updated

10 years ago
Blocks: 323926
Group: security
Flags: in-testsuite?
(Reporter)

Comment 13

10 years ago
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.