Using mouse wheel in edit mail filter window causes crash

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: lohphat, Assigned: mats)

Tracking

({crash, regression, testcase})

Trunk
x86
All
crash, regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040903
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040903

Crash when editing a mail filter

Reproducible: Always
Steps to Reproduce:
1. Edit a mail filter
2. move the mouse pointer to various places in the dialog
3. scroll the wheel
4. Blammo

Actual Results:  
Talkback dialog

Expected Results:  
scrolled the element or nothing depending on focus.

see lohphat@earthlink.net talkback log
Talkbacks can no longer be searched by email, apparently... what was the
incident ID?
(Reporter)

Comment 2

14 years ago
Where would I find that?
Run components/talkback.exe from the Mozilla install dir and look at the list of
reported incidents.  Pick the one corresponding to this bug.
(Reporter)

Comment 4

14 years ago
TB730974K
Full stack follows below.  Looking at the code, it looks like we try to QI
aTargetFrame after we've dispatched a mouse event a bit earlier.  If that
dispatch destroys the frame, we could end up crashing inside QI like this....

Mats, any idea whether that's the problem here?

nsQueryInterface::operator() 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/glue/nsCOMPtr.cpp,
line 52]
nsCOMPtr_base::assign_from_qi 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/glue/nsCOMPtr.cpp,
line 97]
nsEventStateManager::DoScrollText 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 1685]
nsEventStateManager::PostHandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 2058]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5973]
PresShell::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5833]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2295]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2025]
HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp,
line 79]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1097]
nsWindow::DispatchWindowEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1114]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 4682]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 4659]
nsWindow::WindowProc 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1375]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0x89a5 (0x77d489a5)
USER32.dll + 0x89e8 (0x77d489e8)
nsAppShell::DispatchNativeEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsAppShell.cpp,
line 221]
nsXULWindow::ShowModal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsXULWindow.cpp,
line 374]
nsContentTreeOwner::ShowAsModal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsContentTreeOwner.cpp,
line 461]
nsWindowWatcher::OpenWindowJS 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp,
line 787]
GlobalWindowImpl::OpenInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 4676]
GlobalWindowImpl::OpenDialog 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 3416]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2030]
XPC_WN_CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1288]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1282]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3375]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1301]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1378]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
3713]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1346]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 181]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1511]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1585]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2824]
PresShell::HandleDOMEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5998]
nsButtonBoxFrame::MouseClicked 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 178]
nsButtonBoxFrame::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,
line 147]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5968]
PresShell::HandleEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5864]
nsEventStateManager::CheckForAndDispatchClick 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 2919]
nsEventStateManager::PostHandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 1948]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5973]
PresShell::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5833]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2295]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2025]
HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp,
line 79]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1097]
nsWindow::DispatchWindowEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1114]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5361]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5613]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 4119]
nsWindow::WindowProc 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1375]
USER32.dll + 0x8709 (0x77d48709)
USER32.dll + 0x87eb (0x77d487eb)
USER32.dll + 0x89a5 (0x77d489a5)
USER32.dll + 0x89e8 (0x77d489e8)
nsAppShell::Run 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsAppShell.cpp,
line 159]
nsAppShellService::Run 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsAppShellService.cpp,
line 489]
main1 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1331]
main 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1801]
WinMain 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1827]
Assignee: sspitzer → events
Status: UNCONFIRMED → NEW
Component: Filters → Event Handling
Ever confirmed: true
Product: MailNews → Browser
QA Contact: ian
(Assignee)

Comment 6

14 years ago
Created attachment 157879 [details]
Crash data

bz> Mats, any idea whether that's the problem here?

Indeed it is.
(Assignee)

Updated

14 years ago
Assignee: events → mats.palmgren
Severity: normal → critical
Keywords: crash, regression
OS: Windows XP → All
(Assignee)

Comment 7

14 years ago
Regression is from bug 97283, although I think it could also occur
with the old code, but since |aTargetFrame| was less used
it would rarely occur. (I have a fix I'm testing now)
Flags: blocking1.8a4?
(Assignee)

Comment 8

14 years ago
Created attachment 157896 [details] [diff] [review]
Patch rev. 1
(Assignee)

Updated

14 years ago
Attachment #157896 - Flags: review?(bzbarsky)
Comment on attachment 157896 [details] [diff] [review]
Patch rev. 1

Why is it ok to return early here?  Doesn't that skip out on the "passToParent"
block at the end of the function? And wouldn't we want to do that even if the
content went display:none?

I suppose the content could also have been removed from the tree, though... 
I'm not quite sure what GetParentScrollingView would do then.
(Assignee)

Comment 10

14 years ago
Created attachment 157953 [details]
(iframe contents for Testcase #1)
(Assignee)

Comment 11

14 years ago
Created attachment 157954 [details]
Testcase #1
Attachment #157953 - Attachment is obsolete: true
(Assignee)

Comment 12

14 years ago
(In reply to comment #9)
> (From update of attachment 157896 [details] [diff] [review])
> Why is it ok to return early here?

I think that in most cases that would be the wrong to scroll. Since we
have no frame we can't complete the normal ancestor search for the closest
thing to scroll. The "passToParent" block at the end is only for the case
where we should traverse to an enclosing document, e.g. for an IFRAME.

In Testcase #1, when the blue box disappears, the correct thing to scroll
would be the red box, if I remove the early return the event will propagate
to the green box. But this is only for one event, the following events
will go to the red box. IMO, it's better to not scroll than to scroll the
wrong view. (Byt maybe you have a counter-example?)
Comment on attachment 157896 [details] [diff] [review]
Patch rev. 1

OK.  That makes sense.	Add a comment with that explanation before the early
return, ok?
Attachment #157896 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 14

14 years ago
Created attachment 158654 [details] [diff] [review]
Patch rev. 2

Added a comment.
Attachment #157896 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #158654 - Flags: superreview?(bzbarsky)
Attachment #158654 - Flags: review?(bzbarsky)
Comment on attachment 158654 [details] [diff] [review]
Patch rev. 2

r+sr=bzbarsky
Attachment #158654 - Flags: superreview?(bzbarsky)
Attachment #158654 - Flags: superreview+
Attachment #158654 - Flags: review?(bzbarsky)
Attachment #158654 - Flags: review+
(Assignee)

Comment 16

14 years ago
Checked in 2004-09-12 14:24 PDT

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Flags: blocking1.8a4?
Keywords: testcase
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.