Closed Bug 257998 Opened 20 years ago Closed 20 years ago

Using mouse wheel in edit mail filter window causes crash

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: lohphat, Assigned: MatsPalmgren_bugz)

Details

(Keywords: crash, regression, testcase)

Attachments

(3 files, 2 obsolete files)

Crash data
876 bytes, text/plain
Details
Testcase #1
383 bytes, text/html
Details
Patch rev. 2
1.46 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
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?
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.
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
Attached file Crash data — 
bz> Mats, any idea whether that's the problem here?

Indeed it is.
Assignee: events → mats.palmgren
Severity: normal → critical
Keywords: crash, regression
OS: Windows XP → All
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?
Attached patch Patch rev. 1 (obsolete) — — Splinter Review 

    
  

        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.

    
    

    
  

      
      
      
      

        Attached file
          
        (iframe contents for Testcase #1) (obsolete)
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        Testcase #1
        — 
      

    


  

        Attachment #157953 -
        Attachment is obsolete: true

    
    

    
  
(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+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 2
        — 
          — Splinter Review
      

    


  
Added a comment.

        Attachment #157896 -
        Attachment is obsolete: true

    
  

        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+

    
    

    
  
Checked in 2004-09-12 14:24 PDT

-> FIXED
Status: NEW → RESOLVED
Closed: 20 years ago
Flags: blocking1.8a4?
Keywords: testcase
Resolution: --- → FIXED
Component: Event Handling → User events and focus handling

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: