56136 - Composer-Crash at changing Character coding menu in non-meta charset page [@ nsQueryInterface::operator - gkhtml.dll]

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Teruko Kobayashi]

When you open the non-meta charset page in composer and change the character coding menu,
Netscape will crash.

Steps of reproduce
1. Open Composer
2. Open Non-meta charset html file or text file
3. Select menu View|Character coding->More->East Asian->Japanese(Shift_JIS)

Netscape will crash.

Tested 2000-10-11-08 Win32 MN6 build.  This happen in 2000-10-10-08 MN6 build.
This does not happen in 2000-10-08-08 MN6 build and 2000-10-11-09 trunk build.

Talkback incident #18928478

Trigger Type:  Program Crash 
 Trigger Reason:  Access violation 
 Thread ID:   
 Call Stack:    (Signature = 0x00000070 b509c82b) 
     0x00000070 
     nsQueryInterface::operator() 
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 37]
     nsCOMPtr_base::assign_from_helper 
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 66]
     nsInterfaceState::SelectionIsCollapsed 
[d:\builds\seamonkey\mozilla\editor\base\nsInterfaceState.cpp, line 384]
     nsInterfaceState::TimerCallback 
[d:\builds\seamonkey\mozilla\editor\base\nsInterfaceState.cpp, line 283]
     nsInterfaceState::Notify 
[d:\builds\seamonkey\mozilla\editor\base\nsInterfaceState.cpp, line 660]
     nsTimer::Fire 
[d:\builds\seamonkey\mozilla\widget\timer\src\windows\nsTimer.cpp, line 200]
     nsTimerManager::FireNextReadyTimer 
[d:\builds\seamonkey\mozilla\widget\timer\src\windows\nsTimerManager.cpp, line 117]
     nsAppShell::Run 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsAppShell.cpp, line 116]
     nsAppShellService::Run 
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp, line 408]
     Netscp6.exe + 0x1711 (0x00401711) 
     Netscp6.exe + 0x1230 (0x00401230) 
     Netscp6.exe + 0x2aae (0x00402aae) 
     KERNEL32.dll + 0x1bd1e (0x77eebd1e) 
                                                                     
Hotta-san, would you reassign this to the right person?  thanks.

Comment 1

[Frank Tang]

kin- regression. know anything may related to this ?

Comment 2

[nhottanscp]

IQA, could you try 10/09 branch build?

Comment 3

[nhottanscp]

I can reproduce this with my local branch build (pulled today).
Reassign to editor group.
It looks like this started between 10/8 and 10/10.

Comment 4

[nhottanscp]

I cannot reproduce this using 2000-10-09-09-MN6 win32 buld.

Comment 5

[nhottanscp]

I also tried trunk builds, it started from 2000-10-09-21-Mtrunk.
Here is a check in of that day (Hook for tree that closed on 10/10/2000 08:00:07).

http://bonsai.mozilla.org/showcheckins.cgi?&treeid=SeaMonkey&batchid=369

Comment 6

[nhottanscp]

Adding sfraser to cc (just because he had check in to editor that day, don't
know if that's related to this bug).

Comment 7

[Kathleen :Brade]

adding a url above which doesn't have a charset meta tag

Reassign to Simon since this looks like it *might* be related to one of his 
checkins.  We're crashing in nsInterfaceState::TimerCallback and mEditor has been 
deleted.  Perhaps the window is being deconstructed?  Sounds kind of like the 
browse twice crasher... hmmmm...

Comment 8

[Simon Fraser [no longer active]]

nsInterfaceState is being leaked. Investigating

Comment 9

[Simon Fraser [no longer active]]

I understand this problem now. The UI interface updater has a timer that fires 
after the old editor has gone away, but before we have a new one (after the 
charset change). Fix should be pretty easy.

Comment 10

[Simon Fraser [no longer active]]

Accept

Comment 11

[Jim Roskind]

Marking rtm need info.
Please attach a patch, gets reviews, and renominate if still reasonable.

Comment 12

[Simon Fraser [no longer active]]

Patch:

Index: mozilla/editor/base/nsEditorShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/editor/base/nsEditorShell.cpp,v
retrieving revision 1.202.2.4
diff -u -2 -r1.202.2.4 nsEditorShell.cpp
--- nsEditorShell.cpp	2000/10/13 13:33:03	1.202.2.4
+++ nsEditorShell.cpp	2000/10/13 22:40:03
@@ -377,4 +377,10 @@
   // new one.
 
+  nsCOMPtr<nsIEditor> editor(do_QueryInterface(mEditor));
+  if (editor)
+  {
+    editor->PreDestroy();
+  }
+
   // Unload existing stylesheets
   nsCOMPtr<nsIEditorStyleSheets> styleSheets = do_QueryInterface(mEditor);

What this patch does is to call the editor's PreDestroy method, which notifies 
document state listeners that the document is going away. One of those listeners 
is the nsInterfaceState, which responds to this notification by removing the 
timer that's causing this crash.

This is a safe change.

Comment 13

[Akkana Peck]

Looks like a good change, r=akkana.

Comment 14

[kinmoz]

sr=[e-mail]

Comment 15

[Jim Roskind]

I expect this will be approved for checkin this afternoon... but I want the PDT
to discuss the details of approval process first.  Watch for the double-plus
landing permission.
Thanks,
Jim

Comment 16

[Peter Trudelle]

PDT says rtm++, please land on branch and trunk ASAP

Comment 17

[Jay Patel [:jay]]

adding topcrash keyword and [@ nsQueryInterface::operator] for tracking...this 
is a topcrasher for the latest branch builds.

Comment 18

[Simon Fraser [no longer active]]

Fix checked into branch and trunk.

Comment 19

[Teruko Kobayashi]

I verified this in 2000-10-19-08 MN6 Win32, Linux, and Mac build.

Comment 20

[Teruko Kobayashi]

Added vtrunk in keywords.

Comment 21

[Asa Dotzler [:asa]]

Reopening to Resolve as Fixed.  Bugs stay resolved until verified on both the
branch and the trunk.

Comment 22

[Jim Roskind]

marking as fixed (per asa's apparent intention) so that we can verify this on
the branch as well (already verified on the trunk).

Comment 23

[sujay]

this bug is already verified on the branch per teruko's comments.

Someone will have to verify on trunk. Asa ?

Comment 24

[Teruko Kobayashi]

Verified as fixed in 2000-12-12-04 Mtrunk Win32 build, 2000-12-12-08 Mtrunk Mac 
build, and 2000-12-12-09 Mtrunk Linux build.