GPF in MFCEmbed when using Shift F10 Shortcut key

VERIFIED FIXED in mozilla1.0

Status

()

Core
Event Handling
P3
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: kinmoz, Assigned: kinmoz)

Tracking

({access, topembed+})

Trunk
mozilla1.0
x86
Windows 2000
access, topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
This bug was originally reported as bugscape bug 12746.

To reproduce:

1. Run mfcembed.exe
2. Click in the content area.
3. Type Shift-F10 to display a context menu.
4. Click the mouse in the content area to make the context menu go away.

You should then crash with a stack trace similar to:


nsWindow::DispatchMouseEvent(unsigned int 501, unsigned int 197984, nsPoint * 
0x00000000) line 4786 + 3 bytes
ChildWindow::DispatchMouseEvent(unsigned int 501, unsigned int 197984, nsPoint * 
0x00000000) line 4963
nsWindow::ProcessMessage(unsigned int 123, unsigned int 197984, long -1, long * 
0x0012fc1c) line 3604 + 30 bytes
nsWindow::WindowProc(HWND__ * 0x00030560, unsigned int 123, unsigned int 197984, 
long -1) line 1130 + 27 bytes
USER32! 77e12e98()
USER32! 77e139a3()
USER32! 77e1395f()
NTDLL! 77fa032f()
USER32! 77e14ef0()
USER32! 77e12e98()
USER32! 77e16a72()
USER32! 77e16aee()
nsWindow::WindowProc(HWND__ * 0x00030560, unsigned int 260, unsigned int 121, 
long 4456449) line 1137 + 31 bytes
USER32! 77e12e98()
USER32! 77e130e0()
USER32! 77e15824()
CWinThread::Run() line 480 + 11 bytes
CWinApp::Run() line 400
AfxWinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char * 
0x0013335e, int 1) line 49 + 11 bytes
WinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char * 0x0013335e, 
int 1) line 30
WinMainCRTStartup() line 330 + 54 bytes


It crashes at line 4786 of nsWindow.cpp, because of the NS_RELEASE(event.widget) 
call.

The event we're trying to release the widget from, did have a valid widget 
pointer before the event was dispatched, but it is being null'd out by some code 
in nsEventListenerManager::HandleEvent() that pinkerton added back in May of 
2001:


          // If we're here because of the key-equiv for showing context menus, 
we
          // have to twiddle with the NS event to make sure the context menu 
comes
          // up in the upper left of the relevant content area before we create
          // the DOM event. Since we never call InitMouseEvent() on the event, 
          // the client X/Y will be 0,0. We can make use of that if the widget 
is null.
          if ( aEvent->message == NS_CONTEXTMENU_KEY )
            NS_IF_RELEASE(((nsGUIEvent*)aEvent)->widget);   // nulls out widget  
                        

That said, I think we can fix this by using NS_IF_RELEASE(event.widget) instead 
of NS_RELEASE(event.widget) in nsWindow::DispatchMouseEvent().
(Assignee)

Comment 1

16 years ago
Created attachment 77065 [details] [diff] [review]
Minimal Patch to avoid the crash. (Based on TRUNK)

Here's the minimal patch to fix the crash. (Based on the TRUNK) This crash is
also happening on the 0.9.9 branch.
(Assignee)

Comment 2

16 years ago
Transfering keywords from bugscape bug 12746.
Status: NEW → ASSIGNED
Keywords: access, edt0.9.9, topembed+
Priority: -- → P3
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 3

16 years ago
Comment on attachment 77065 [details] [diff] [review]
Minimal Patch to avoid the crash. (Based on TRUNK)

Transfering r=pinkerton from bugscape bug 12746.
Attachment #77065 - Flags: review+

Comment 4

16 years ago
See Bugscape:
http://bugscape.netscape.com/show_bug.cgi?id=12746
(Assignee)

Updated

16 years ago
Whiteboard: FIX IN HAND, need sr= and a=

Comment 5

16 years ago
adding + to edt0.9.9. let's get this sr'd and in. once it's in, please add
"fixed0.9.9" to the keyword field.
Keywords: edt0.9.9 → edt0.9.9+

Comment 6

16 years ago
Comment on attachment 77065 [details] [diff] [review]
Minimal Patch to avoid the crash. (Based on TRUNK)

yes, yes - check this in!  sr=attinasi
Attachment #77065 - Flags: superreview+
(Assignee)

Updated

16 years ago
Whiteboard: FIX IN HAND, need sr= and a= → FIX IN HAND, need a=
(Assignee)

Comment 7

16 years ago
Fix checked into the MOZILLA_0_9_9_BRANCH:

  mozilla/widget/src/windows/nsWindow.cpp  revision 3.404.2.2

Requesting checkin permission from adt and drivers for checkin on trunk.
Keywords: fixed0.9.9
(Assignee)

Updated

16 years ago
Keywords: adt1.0.0

Comment 8

16 years ago
adt1.0.0+ per ADT.
Keywords: adt1.0.0 → adt1.0.0+

Comment 9

16 years ago
Comment on attachment 77065 [details] [diff] [review]
Minimal Patch to avoid the crash. (Based on TRUNK)

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #77065 - Flags: approval+
(Assignee)

Comment 10

16 years ago
Fix checked into the TRUNK:

    mozilla/widget/src/windows/nsWindow.cpp  revision 3.410
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: FIX IN HAND, need a=

Comment 11

16 years ago
-- Verified it on mfcEmbed 2002-04-02-23-0.9.9ec. Patch works fine.
-- Verified it on todays trunk build. Patch works fine.
-- Marking bug as verified.
Status: RESOLVED → VERIFIED

Updated

16 years ago
Keywords: fixed1.0.0

Comment 12

16 years ago
Verified on the branch build 2002-05-22-08-1.0.0 on Windows 2000.
adding "verified1.0.0" to the keyword
Keywords: verified1.0.0
QA Contact: madhur → rakeshmishra
You need to log in before you can comment on or make changes to this bug.