71032 - printing www.msn.com crashes browser

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P1)

Description

[dcone (gone)]

printing www.msn.com crashes browser.  Looks like some sort of delete problem in 
layout. I have to reduce this test case.

Comment 1

[Kevin McCluskey (gone)]

Today's build does on WINNT does not crash for me. I think [e-mail]
recently checked in a fix related to blockcode reparenting views which may have
fixed this.
Marking fixed.

Comment 2

[Christine Hoffman]

Tested on Win98 using 3/20 commercial build. Crash. Here is the stack trace:

Stack Trace

0x0fdefe49 
GKLAYOUT.DLL + 0x60196 (0x60340196) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x60196 (0x60340196) 
GKLAYOUT.DLL + 0x64dee (0x60344dee) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x60196 (0x60340196) 
GKLAYOUT.DLL + 0x90074 (0x60370074) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x30f11 (0x60310f11) 
GKLAYOUT.DLL + 0x296ee (0x603096ee) 
GKLAYOUT.DLL + 0x30f11 (0x60310f11) 
GKLAYOUT.DLL + 0x296ee (0x603096ee) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x4be28 (0x6032be28) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x49d8d (0x60329d8d) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x296e5 (0x603096e5) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x4be28 (0x6032be28) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x49d8d (0x60329d8d) 
GKLAYOUT.DLL + 0x30f11 (0x60310f11) 
GKLAYOUT.DLL + 0x296ee (0x603096ee) 
GKLAYOUT.DLL + 0x30f11 (0x60310f11) 
GKLAYOUT.DLL + 0x296ee (0x603096ee) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x5579 (0x602e5579) 
GKLAYOUT.DLL + 0x1d8b2 (0x602fd8b2) 
GKLAYOUT.DLL + 0x87de6 (0x60367de6) 
GKLAYOUT.DLL + 0x26d22 (0x60306d22) 
GKLAYOUT.DLL + 0x1707f (0x602f707f) 
GKLAYOUT.DLL + 0x16dde (0x602f6dde) 
GKLAYOUT.DLL + 0x8e990 (0x6036e990) 
nsEventQueueServiceImpl::GetThreadEventQueue
[d:\builds\seamonkey\mozilla\xpcom\threads\nsEventQueueService.cpp, line 356] 
GKCONTENT.DLL + 0x5971 (0x601c5971) 
GKCONTENT.DLL + 0x6446 (0x601c6446) 
GKCONTENT.DLL + 0x6b7f (0x601c6b7f) 
LocationAssign [d:\builds\seamonkey\mozilla\dom\src\base\nsJSLocation.cpp, line 
347] 
SetWindowProperty [d:\builds\seamonkey\mozilla\dom\src\base\nsJSWindow.cpp, line 
1456] 
js_InitAtomState [d:\builds\seamonkey\mozilla\js\src\jsatom.c, line 272] 

Reopening bug. Also crashing on Mac

Comment 3

[lchiang]

we have a dupe bug entered on this under 
http://bugscape.netscape.com/show_bug.cgi?id=4284

You may want to check some comments in that bug report.

Comment 4

[Brendan Eich [:brendan]]

Can we get the bugscape bug dup'ed against this one, and any relevant and
non-confidential comments copied into this bug?  No point in reporting a bug
against open source using a closed bugsystem.

/be

Comment 5

[chris hofmann]

every thing that we know about the problem has been moved here. 

Comment 6

[dcone (gone)]

Attachment: reduced test case

Comment 7

[dcone (gone)]

Attachment: better test case

Comment 8

[dcone (gone)]

Attachment: Better still test case

Comment 9

[Kevin McCluskey (gone)]

Attachment: Patch file that stops the crashing

Comment 10

[Kevin McCluskey (gone)]

Attachment: Cleaned up version of printpatch

Comment 11

[Kevin McCluskey (gone)]

Ok, I think I tracked down the problem. The patch file I attached does three things:

- removes some earlier hack that prevented views from being reparented
- Checks the floater list to see if any of it's frames have views which need to
be reparented
- Detaches the view using using the pointer returned from GetParent instead of
the incorrect value passed as an argument.

Some layout guys need to look this over closely and it needs lots and lots of
testing.

Comment 12

[Chris Waterson]

Didn't look at this too closely, but should we worry about absolutely positioned
frames, too?

Comment 13

[Marek Z. Jeziorek]

SPAM: thanks for tracking it down and proposing fix so quickly. You rock!

Comment 14

[Kevin McCluskey (gone)]

"should we worry about absolutely positioned
frames, too?"

Chris: I was wondering about that. Maybe I should cycle through all of the
additional frame lists looking for views.  I don't think we ever want to move a
frame in the normal frame lists or any of the additional frame lists without
moving their views. If we don't move the views then the frame tree and the view
tree will be inconsistent. What do you think?

Comment 15

[Chris Waterson]

Yeah: I've been chasing bugs like that for months in the block-in-inline code.
So maybe go ahead and look through the other frame lists as well.

Comment 16

[Kevin McCluskey (gone)]

Attachment: Patch which looks for views in all of the child lists, not just the normal, overflow and floater lists

Comment 17

[Kevin McCluskey (gone)]

The latest patch (28544) crashes when I print www.icq.com. It doesn't crash when
I use  original patch(28449).  With the latest patch the view hierarchy is
messed up for www.icq.com. Going through all of the child frames must migrate
views that should not be migrated (Possibly the combobox?)

I'm going to test just the original patch, since it fixes the msn.com crash and
doesn't seem to cause any additional problems.

I'll file a separate bug to investigate why we can't cycle through all of the
child list frames when reparenting views.

Comment 18

[Chris Waterson]

ok, sr=waterson on patch 28449.

Comment 19

[Kevin McCluskey (gone)]

Taking bug.

Comment 20

[dcone (gone)]

r=dcone

Comment 21

[Kevin McCluskey (gone)]

I didn't crash printing 50 of the top 100 using patch 28449. Don also ran his
printing tests and approx 70% of the top 100 using the same patch and it didn't
crash.

Fixed checked into the trunk.

Comment 22

[Kevin McCluskey (gone)]

I filed a new bug to cover the fact that we do not cycle through all of the
additional child lists for the frame when reparenting the views. bug 73188

Comment 23

[Kevin McCluskey (gone)]

I checked in the patch 28449 to both the trunk and the branch
(MOZILLA_0_8_1_BRANCH).

Comment 24

[Christine Hoffman]

Tested with 3/26 commercial build. No crash. Verified bug fixed.