243203 - Crash removing address line in message compose window [@ nsCSSFrameConstructor::AttributeChanged]

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Tuukka Tolvanen (sp3000)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a) Gecko/20040424 Firefox/0.8.0+
Build Identifier: linux gtk2 thunderbird trunk cvs 20040510

Not quite 100% reproducible, but crashes more often than not

Reproducible: Sometimes
Steps to Reproduce:
1. select a message, reply all
2. click at end of last address line text, backspace until blank
3. backspace
Actual Results:  
crash

Expected Results:  
removed address line

Comment 1

[Tuukka Tolvanen (sp3000)]

Attachment: stack

0.6 wfm
    2004-05-04-01-trunk wfm
    2004-05-05-12-trunk crashes

Comment 2

[Tuukka Tolvanen (sp3000)]

backing out 2004-05-04 04:13 varga%nixcorp.com bug 241068 fixes this crash for me

Comment 3

[Scott MacGregor]

thanks for tracking this down. Jan can you take a look at why your change caused
this crash for us?

Comment 4

[Scott MacGregor]

*** Bug 243189 has been marked as a duplicate of this bug. ***

Comment 5

[Jan Varga [:janv]]

The problem is that I'm not able to reproduce with my Linux build.
It seems that CSS frame ctor is trying to call AttributeChanged() method for a
deleted frame.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Same here with version 0.6+ (20040511)

If I open the compose window and delete an address line Thunderbird crashes
immediately. A workaround to prevent the crash is to manually set the cursor
into the last address line and click the down key. After a new line is inserted
at the end I can remove any other address from the list.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I hit

###!!! ASSERTION: frame was not removed from primary frame map before
destruction or was readded to map after being removed:
'!PL_DHASH_ENTRY_IS_BUSY(entry) || entry->frame != aFrame', file
/builds/trunk/mozilla/layout/html/base/src/nsFrameManager.cpp, line 796

six times before crashing.

Comment 8

[Frank Wein [:mcsmurf]]

*** Bug 243415 has been marked as a duplicate of this bug. ***

Comment 9

[Scott MacGregor]

Jan, can we back out your change that introduced this crash until we can fix the
regression? 

Comment 10

[Jan Varga [:janv]]

Well, I don't mind, but it will crash in autocomplete again.

Comment 11

[Scott MacGregor]

ooh that crash was definetly more annoying than this one :)

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

OS -> ALL because it crashes for me on Win2k

Comment 13

[[e-mail]]

Once I actually managed to get Mozilla to crash opening the Compose window.

The function on the top of JS stack was awCalcContentHeight().

The C++ stack was mostly concerned with constructing frames. The bottommost
frames were from a tree col frame trying to locate the tree so as to invalidate
its columns. Unfortunately the tree's frame had been deleted.

Comment 14

[lohphat]

I wonder if it's hit the trunk as well as I just posted Bug 243454 for the same
behavior on XP with Moz 1.8a.

Comment 15

[Scott MacGregor]

(In reply to comment #14)
> I wonder if it's hit the trunk as well as I just posted Bug 243454 for the same
> behavior on XP with Moz 1.8a.
yes this has hit the trunk. This crash is trunk only.

Comment 16

[Scott MacGregor]

*** Bug 243454 has been marked as a duplicate of this bug. ***

Comment 17

[Charles Fenwick]

*** Bug 243456 has been marked as a duplicate of this bug. ***

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Moving to MailNews/Composition because of comment 14 and bug 243454.

Comment 19

[Jan Varga [:janv]]

Working on a real fix for crashes described in this bug and bug 241068.
Hope, this one will fix it once for all.

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 243514 has been marked as a duplicate of this bug. ***

Comment 21

[Jan Varga [:janv]]

Attachment: fix

Comment 22

[[e-mail]]

Comment on attachment 148441 [details] [diff] [review]
fix

While I'm waiting for my rebuild, would you mind explaining why nsITreeColumns
is safer than nsITreeBoxObject?

Comment 23

[Jan Varga [:janv]]

For some reason, getting tree box object at col "destroy" time causes assertions
pointed out in comment 7 and various crashes after that. Keeping columns in the
col frame seems to be reasonable since we need it more often than tree box object.
I think, there could be an edge case when EnsureColumns() in Init() method
doesn't get columns because there's no body frame constructed yet. So this fix
isn't perfect. What we need to do next is removing EnsureColumns() and calling
colFrame->SetColumns(this) from nsTreeColumns::EnsureColumns() or
mFrame->SetColumns(mColumns) from nsTreeColumn::nsTreeColumns.
But let's check this in ASAP.

Comment 24

[Jan Varga [:janv]]

(In reply to comment #23)

> What we need to do next is removing EnsureColumns() and calling
> colFrame->SetColumns(this) from nsTreeColumns::EnsureColumns() or
> mFrame->SetColumns(mColumns) from nsTreeColumn::nsTreeColumns.

We can't remove EnsureColumns() because some cols can be added lazily.
Need to figure out something better. Anyway, the fix should be ok for now.

Comment 25

[[e-mail]]

(In reply to comment #13)
>Once I actually managed to get Mozilla to crash opening the Compose window.
"False" alarm, that crash was caused by this style in my userChrome.css:
scrollbar[maxpos="0"] > slider > thumb { display: none; }

Comment 26

[[e-mail]]

Attachment: Fix point of crash

Rather than relying on an obscure side-effect of attachment 148441 [details] [diff] [review] I suggest
this patch is actually a better fix for the bug for the following reasons:

1. By clearing the current item before it is deleted this fixes a crash caused
by trying to remove the current="true" attribute on the item being deleted.

2. Since the listbox.xml code already fires the select event before the item is
deleted this fixes a crash caused by trying to fire another select event on a
listitem after it has been deleted, rather than on the listbox.

Comment 27

[[e-mail]]

Comment on attachment 148441 [details] [diff] [review]
fix

Hmm... so perhaps the old code was doing something weird with frames which was
triggering the crash removing the parent listitem?

Comment 28

[Scott MacGregor]

*** Bug 243552 has been marked as a duplicate of this bug. ***

Comment 29

[Scott MacGregor]

*** Bug 243614 has been marked as a duplicate of this bug. ***

Comment 30

[Jan Varga [:janv]]

Comment on attachment 148441 [details] [diff] [review]
fix

checked in

Comment 31

[Leonel]

I still get a crash with the steps outlined on bug 243614, even with version 0.6+ (20040517). I 
sent a new TB report, ID is TB49766Z. 

Comment 32

[Jan Varga [:janv]]

Is it really the same crash?
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB47279Q

Comment 33

[Leonel]

(In reply to comment #32)
> Is it really the same crash?

I don't know, but now it is working on version 0.6+ (20040518).

Comment 34

[Jan Varga [:janv]]

(In reply to comment #33)
> I don't know, but now it is working on version 0.6+ (20040518).

Do you mean, that it doesn't crash anymore?

Comment 35

[Leonel]

(In reply to comment #34)
> > I don't know, but now it is working on version 0.6+ (20040518).
> 
> Do you mean, that it doesn't crash anymore?

Yes. It crashed everytime on 20040517, but did not crash once on 20040518.

Comment 36

[OstGote!]

This patch fixed my crashes on Mozilla Mail&News, despite the fact, that I
always had a stack signature nsCSSFrameConstructor::FindFrameWithContent (see
bug 203041 comment 43). Tested with trunk build 2004051709. Thanks!

Comment 37

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 244085 has been marked as a duplicate of this bug. ***

Comment 38

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 244636 has been marked as a duplicate of this bug. ***

Comment 39

[Phillip M. Jones, C.E.T.]

the last version of Mozilla 1.8a (Build 2004052608) nightly I downloaded, no
longer has this crash problem. Thanks for fixing in timely fashion.

Comment 40

[Stephen Donner [:stephend] Not actively reading bugmail]

Tuukka, are you still seeing this with a nightly build?

Comment 41

[Tuukka Tolvanen (sp3000)]

No, I don't see the crash anymore in linux thunderbird 2004-07-14-03-trunk.
marking fixed

Comment 42

[Stephen Donner [:stephend] Not actively reading bugmail]

Me neither.

Verified FIXED using 2004-07-25 in Windows XP.