Closed Bug 455871 Opened 12 years ago Closed 12 years ago

[mac] Regression: drag & drop is not working at all after some-time of browser usage

Categories

(Core :: DOM: Drag & Drop, defect, P2, major)

x86
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: mano, Assigned: mayhemer)

References

Details

(Keywords: regression, verified1.9.1)

Attachments

(3 files, 3 obsolete files)

In the latest mac nightly builds, drag and drop operations stop working some time the browser is used, and they remain broken until a restart of the browser.
Flags: blocking1.9.1?
This is hitting me and roc all the time.  Usually I manage to get in 2-3 drags, and then nothing works.  That is, the drags don't even start.  This applies to tabs, links, selected text, everything.

Not sure whether this should block beta...
I did some testing to see if I could reproduce this, it is hard to test because bug 454324 (and/or bug 458048) crashes the browser every few times I drag something.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Assignee: nobody → smichaud
Steven, before I noticed you took this bug, I played with it a few moments. I can reproduce the problem (absolutely no idea how) and it is broken here: nsEventStateManager::DoDefaultDragStart, line 2216. There is left current drag session (it has not been freed).

If you want I can work on this bug.
Assignee: smichaud → honzab
STR:
1. http://en-us.www.mozilla.com/en-US/firefox/central/
2. drag "mozilla" link from above (any of them) on the "Work" link (or any other) on one of the tabs
3. release mouse button and _do not move the mouse_
4. now press the mouse button again and try to drag "Work" link, it won't work from that moment

continuing with the code analyzes now.
Note: instead of a on link you can also drop the object on e.g. a selected text to reproduce. Actually, it can be anything that can be dragged.
These are backtraces when you press the mouse button in step 4 of the STR from comment 5:

Invocation of nsSynthMouseMoveEvent:

#0  nsViewManager::SynthesizeMouseMove (this=0x22a4430, aFromScroll=0) at /mozilla/HG/src/view/src/nsViewManager.cpp:2317
#1  0x10c6da9b in PresShell::DidDoReflow (this=0x2be1600) at /mozilla/HG/src/layout/base/nsPresShell.cpp:6251
#2  0x10c7b431 in PresShell::ProcessReflowCommands (this=0x2be1600, aInterruptible=0) at /mozilla/HG/src/layout/base/nsPresShell.cpp:6435
#3  0x10c7b6c0 in PresShell::DoFlushPendingNotifications (this=0x2be1600, aType=Flush_Layout, aInterruptibleReflow=0) at /mozilla/HG/src/layout/base/nsPresShell.cpp:4563
#4  0x10c7b8a8 in PresShell::FlushPendingNotifications (this=0x2be1600, aType=Flush_Layout) at /mozilla/HG/src/layout/base/nsPresShell.cpp:4500
#5  0x10ea43ba in nsDocument::FlushPendingNotifications (this=0x2b23600, aType=Flush_Layout) at /mozilla/HG/src/content/base/src/nsDocument.cpp:6227
#6  0x10c700e4 in PresShell::ScrollContentIntoView (this=0x2be1600, aContent=0x2d4a0bb0, aVPercent=-2, aHPercent=-2) at /mozilla/HG/src/layout/base/nsPresShell.cpp:3971
#7  0x10f6696a in nsHTMLAnchorElement::SetFocus (this=0x2d4a0bb0, aPresContext=0x2b8ca00) at /mozilla/HG/src/content/html/content/src/nsHTMLAnchorElement.cpp:264
#8  0x10f2a402 in nsEventStateManager::ChangeFocusWith (this=0x2d486270, aFocusContent=0x2d4a0bb0, aFocusedWith=eEventFocusedByMouse) at /mozilla/HG/src/content/events/src/nsEventStateManager.cpp:3906
#9  0x10f368be in nsEventStateManager::PostHandleEvent (this=0x2d486270, aPresContext=0x2b8ca00, aEvent=0xbfffee24, aTargetFrame=0x2c5fc78, aStatus=0xbfffeb98, aView=0x2d4b8aa0) at /mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2771
#10 0x10c77f26 in PresShell::HandleEventInternal (this=0x2be1600, aEvent=0xbfffee24, aView=0x2d4b8aa0, aStatus=0xbfffeb98) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5946
#11 0x10c7819d in PresShell::HandlePositionedEvent (this=0x2be1600, aView=0x2d4b8aa0, aTargetFrame=0x2c5fdb8, aEvent=0xbfffee24, aEventStatus=0xbfffeb98) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#12 0x10c7a21b in PresShell::HandleEvent (this=0x2be1600, aView=0x2d4b8aa0, aEvent=0xbfffee24, aEventStatus=0xbfffeb98) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#13 0x11090ffa in nsViewManager::HandleEvent (this=0x22a4430, aView=0x2d4b8aa0, aPoint=@0xbfffec78, aEvent=0xbfffee24, aCaptured=0) at /mozilla/HG/src/view/src/nsViewManager.cpp:1389
#14 0x11091f28 in nsViewManager::DispatchEvent (this=0x22a4430, aEvent=0xbfffee24, aStatus=0xbfffed08) at /mozilla/HG/src/view/src/nsViewManager.cpp:1348
#15 0x11088509 in HandleEvent (aEvent=0xbfffee24) at /mozilla/HG/src/view/src/nsView.cpp:167
#16 0x0bfb5024 in nsChildView::DispatchEvent (this=0x2faa0be0, event=0xbfffee24, aStatus=@0xbfffeddc) at /mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1791
#17 0x0bfa7a36 in nsChildView::DispatchWindowEvent (this=0x2faa0be0, event=@0xbfffee24) at /mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1804
#18 0x0bfb5935 in -[ChildView mouseDown:] (self=0x30595f30, _cmd=0x90ab7a3c, theEvent=0x305f07b0) at /mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3098
#19 0x93374df3 in -[NSWindow sendEvent:] ()
#20 0x0bfa6a71 in -[NSWindow(MethodSwizzling) nsCocoaWindow_NSWindow_sendEvent:] (self=0x33b8c760, _cmd=0x90ac34c4, anEvent=0x305f07b0) at /mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:2201
#21 0x0bf9e2e7 in -[ToolbarWindow sendEvent:] (self=0x33b8c760, _cmd=0x90ac34c4, anEvent=0x305f07b0) at /mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:1895
#22 0x93366d8c in -[NSApplication sendEvent:] ()
#23 0x932918e7 in -[NSApplication run] ()
#24 0x0bf99bf5 in nsAppShell::Run (this=0x2226300) at /mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#25 0x0f4a60f7 in nsAppStartup::Run (this=0x22426f0) at /mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#26 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at /mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#27 0x000027a4 in main (argc=5, argv=0xbffff9a4) at /mozilla/HG/src/browser/app/nsBrowserApp.cpp:156

And this is how the nsSynthMouseMoveEvent is handled: it calls StartDragSession() that sets the mDoingDrag member to TRUE. When GetCurrentSession is called it returns a valid pointer and the code believes there has already been a drag session in progress. There is no more a call to EndDragSession().

#0  nsBaseDragService::StartDragSession (this=0x10bae370) at /mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:334
#1  0x0bf966df in nsDragService::InvokeDragSession (this=0x10bae370, aDOMNode=0x2d4a0bcc, aTransferableArray=0x34d7be70, aDragRgn=0x0, aActionType=7) at /mozilla/HG/src/widget/src/cocoa/nsDragService.mm:301
#2  0x0bfe23e7 in nsBaseDragService::InvokeDragSessionWithImage (this=0x10bae370, aDOMNode=0x2d4a0bcc, aTransferableArray=0x34d7be70, aRegion=0x0, aActionType=7, aImage=0x0, aImageX=0, aImageY=0, aDragEvent=0x37a76260, aDataTransfer=0x34d7d3a0) at /mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:275
#3  0x10f2d483 in nsEventStateManager::DoDefaultDragStart (this=0x2d486270, aPresContext=0x2b8ca00, aDragEvent=0xbfffdbc0, aDataTransfer=0x34d7d3a0, aDragTarget=0x2d4a0bb0, aIsSelection=0) at /mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2320
#4  0x10f2dc31 in nsEventStateManager::GenerateDragGesture (this=0x2d486270, aPresContext=0x2b8ca00, aEvent=0xbfffe340) at /mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2075
#5  0x10f34427 in nsEventStateManager::PreHandleEvent (this=0x2d486270, aPresContext=0x2b8ca00, aEvent=0xbfffe340, aTargetFrame=0x2f607c4, aStatus=0xbfffe1b8, aView=0x2d4b8aa0) at /mozilla/HG/src/content/events/src/nsEventStateManager.cpp:859
#6  0x10c77cd3 in PresShell::HandleEventInternal (this=0x2be1600, aEvent=0xbfffe340, aView=0x2d4b8aa0, aStatus=0xbfffe1b8) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5915
#7  0x10c7819d in PresShell::HandlePositionedEvent (this=0x2be1600, aView=0x2d4b8aa0, aTargetFrame=0x2f607c4, aEvent=0xbfffe340, aEventStatus=0xbfffe1b8) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#8  0x10c7a21b in PresShell::HandleEvent (this=0x2be1600, aView=0x2d4b8aa0, aEvent=0xbfffe340, aEventStatus=0xbfffe1b8) at /mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#9  0x11090ffa in nsViewManager::HandleEvent (this=0x22a4430, aView=0x2d4b8aa0, aPoint=@0xbfffe298, aEvent=0xbfffe340, aCaptured=0) at /mozilla/HG/src/view/src/nsViewManager.cpp:1389
#10 0x11091f28 in nsViewManager::DispatchEvent (this=0x22a4430, aEvent=0xbfffe340, aStatus=0xbfffe390) at /mozilla/HG/src/view/src/nsViewManager.cpp:1348
#11 0x110906bc in nsViewManager::ProcessSynthMouseMoveEvent (this=0x22a4430, aFromScroll=0) at /mozilla/HG/src/view/src/nsViewManager.cpp:2411
#12 0x1150485c in nsSynthMouseMoveEvent::Run (this=0x34debb10) at /mozilla/HG/src/view/src/nsViewManager.cpp:2306
#13 0x01648db1 in nsThread::ProcessNextEvent (this=0x2212410, mayWait=0, result=0xbfffe4b4) at /mozilla/HG/src/xpcom/threads/nsThread.cpp:510
#14 0x015ebf9b in NS_ProcessPendingEvents_P (thread=0x2212410, timeout=20) at nsThreadUtils.cpp:180
#15 0x0bfda9ef in nsBaseAppShell::NativeEventCallback (this=0x2226300) at /mozilla/HG/src/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#16 0x0bf9b237 in nsAppShell::ProcessGeckoEvents (aInfo=0x2226300) at /mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:302
#17 0x9082bf06 in CFRunLoopRunSpecific ()
#18 0x9082ba42 in CFRunLoopRunInMode ()
#19 0x92df2878 in RunCurrentEventLoopInMode ()
#20 0x92df1eb9 in ReceiveNextEventCommon ()
#21 0x92df1dd9 in BlockUntilNextEventMatchingListInMode ()
#22 0x93297f45 in _DPSNextEvent ()
#23 0x93297b37 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#24 0x932918c4 in -[NSApplication run] ()
#25 0x0bf99bf5 in nsAppShell::Run (this=0x2226300) at /mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#26 0x0f4a60f7 in nsAppStartup::Run (this=0x22426f0) at /mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#27 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at /mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#28 0x000027a4 in main (argc=5, argv=0xbffff9a4) at /mozilla/HG/src/browser/app/nsBrowserApp.cpp:156

If someone can give me a hint here, I would be pleased. I'm not that much familiar with this code.
Attached patch fix, v1 (obsolete) — Splinter Review
Problem is, when the second drag is about to start, synthetic mouse move event is fired what invokes the nsBaseDragService::StartDragSession thanks this condition (nsEventStateManager.cpp:1993):

    nsRect tmpRect;
    aEvent->widget->WidgetToScreen(nsRect(aEvent->refPoint, nsSize(1, 1)),
                                   tmpRect);
    nsPoint pt = tmpRect.TopLeft();
    if (PR_ABS(pt.x - mGestureDownPoint.x) > pixelThresholdX ||
        PR_ABS(pt.y - mGestureDownPoint.y) > pixelThresholdY) {
       ...
    }

'pt' is filled by the old un-updated mMouseLocation at nsViewManager.cpp:2389, exceeding the threshold. The mMouseLocation member contains position of the first drag start. It was not updated if mouse is not moved between mouse-up and mouse-down - the drag operation is completely handled inside of nsEventStateManager::GenerateDragGesture on mac.

So, I added update of the mMouseLocation also when mouse down event is fired. This fixes the problem.
Attachment #343288 - Flags: review?(roc)
Comment on attachment 343288 [details] [diff] [review]
fix, v1

Thanks for taking care of this!

Should we also add NS_MOUSE_BUTTON_UP here? that could be in this patch or a separate patch.
Attachment #343288 - Flags: superreview+
Attachment #343288 - Flags: review?(roc)
Attachment #343288 - Flags: review+
Attached patch fix, v2 (obsolete) — Splinter Review
Unfortunatelly my further testing showed another problem, when GenerateDragGesture is called from mouseMoved. 

This is back trace of correct drag on mac, note it is all handled inside of GenerateDragGesture method:

Entering GenerateDragGesture
#0  nsBaseDragService::StartDragSession (this=0x34509760) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:334
#1  0x271946df in nsDragService::InvokeDragSession (this=0x34509760, aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aDragRgn=0x0, aActionType=7) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:301
#2  0x271e03e7 in nsBaseDragService::InvokeDragSessionWithImage (this=0x34509760, aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aRegion=0x0, aActionType=7, aImage=0x0, aImageX=0, aImageY=0, aDragEvent=0x387aab30, aDataTransfer=0x2e07c1b0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:275
#3  0x0c2a2463 in nsEventStateManager::DoDefaultDragStart (this=0x2e0f3fb0, aPresContext=0x2c61a00, aDragEvent=0xbfffe5b0, aDataTransfer=0x2e07c1b0, aDragTarget=0x2ef6d910, aIsSelection=0) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2329
#4  0x0c2a2c11 in nsEventStateManager::GenerateDragGesture (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2084
#5  0x0c2a9407 in nsEventStateManager::PreHandleEvent (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffee34, aTargetFrame=0x2df8274, aStatus=0xbfffeba8, aView=0x2e0ea260) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:859
#6  0x0bfeccb3 in PresShell::HandleEventInternal (this=0x2b42800, aEvent=0xbfffee34, aView=0x2e0ea260, aStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5915
#7  0x0bfed17d in PresShell::HandlePositionedEvent (this=0x2b42800, aView=0x2e0ea260, aTargetFrame=0x2df8274, aEvent=0xbfffee34, aEventStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#8  0x0bfef1fb in PresShell::HandleEvent (this=0x2b42800, aView=0x2e0ea260, aEvent=0xbfffee34, aEventStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#9  0x0c405fda in nsViewManager::HandleEvent (this=0x2e0ea200, aView=0x2e0ea260, aPoint=@0xbfffec88, aEvent=0xbfffee34, aCaptured=0) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1390
#10 0x0c406f15 in nsViewManager::DispatchEvent (this=0x2e0ea200, aEvent=0xbfffee34, aStatus=0xbfffed18) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1349
#11 0x0c3fd4e9 in HandleEvent (aEvent=0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsView.cpp:167
#12 0x271b3024 in nsChildView::DispatchEvent (this=0x2ef69960, event=0xbfffee34, aStatus=@0xbfffedec) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1791
#13 0x271a5a36 in nsChildView::DispatchWindowEvent (this=0x2ef69960, event=@0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1804
#14 0x271b476a in -[ChildView mouseDragged:] (self=0x2ef69a00, _cmd=0x90ab7aac, theEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3341
#15 0x93375625 in -[NSWindow sendEvent:] ()
#16 0x271a4a71 in -[NSWindow(MethodSwizzling) nsCocoaWindow_NSWindow_sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:2201
#17 0x2719c2e7 in -[ToolbarWindow sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:1895
#18 0x93366d8c in -[NSApplication sendEvent:] ()
#19 0x932918e7 in -[NSApplication run] ()
#20 0x27197bf5 in nsAppShell::Run (this=0x22294e0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#21 0x2a0f50f7 in nsAppStartup::Run (this=0x22458d0) at /Users/starapica/Documents/mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#22 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at /Users/starapica/Documents/mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#23 0x000027a4 in main (argc=5, argv=0xbffff9a4) at /Users/starapica/Documents/mozilla/HG/src/browser/app/nsBrowserApp.cpp:156
#0  nsBaseDragService::EndDragSession (this=0x34509760, aDoneDrag=1) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:342
#1  0x27191dff in nsDragService::EndDragSession (this=0x34509760, aDoneDrag=1) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:579
#2  0x271ac92b in -[ChildView draggedImage:endedAt:operation:] (self=0x2ef69a00, _cmd=0x90aa6314, anImage=0x3874be10, aPoint={x = 906, y = 424}, operation=0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:5914
#3  0x9357ee4a in -[NSFilePromiseDragSource draggedImage:endedAt:operation:] ()
#4  0x9354db35 in -[NSCoreDragManager _dragUntilMouseUp:accepted:] ()
#5  0x9354cee7 in -[NSCoreDragManager dragImage:fromWindow:at:offset:event:pasteboard:source:slideBack:] ()
#6  0x9354c93f in -[NSWindow(NSDrag) dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#7  0x27194830 in nsDragService::InvokeDragSession (this=0x34509760, aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aDragRgn=0x0, aActionType=7) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:313
#8  0x271e03e7 in nsBaseDragService::InvokeDragSessionWithImage (this=0x34509760, aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aRegion=0x0, aActionType=7, aImage=0x0, aImageX=0, aImageY=0, aDragEvent=0x387aab30, aDataTransfer=0x2e07c1b0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:275
#9  0x0c2a2463 in nsEventStateManager::DoDefaultDragStart (this=0x2e0f3fb0, aPresContext=0x2c61a00, aDragEvent=0xbfffe5b0, aDataTransfer=0x2e07c1b0, aDragTarget=0x2ef6d910, aIsSelection=0) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2329
#10 0x0c2a2c11 in nsEventStateManager::GenerateDragGesture (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2084
#11 0x0c2a9407 in nsEventStateManager::PreHandleEvent (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffee34, aTargetFrame=0x2df8274, aStatus=0xbfffeba8, aView=0x2e0ea260) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:859
#12 0x0bfeccb3 in PresShell::HandleEventInternal (this=0x2b42800, aEvent=0xbfffee34, aView=0x2e0ea260, aStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5915
#13 0x0bfed17d in PresShell::HandlePositionedEvent (this=0x2b42800, aView=0x2e0ea260, aTargetFrame=0x2df8274, aEvent=0xbfffee34, aEventStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#14 0x0bfef1fb in PresShell::HandleEvent (this=0x2b42800, aView=0x2e0ea260, aEvent=0xbfffee34, aEventStatus=0xbfffeba8) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#15 0x0c405fda in nsViewManager::HandleEvent (this=0x2e0ea200, aView=0x2e0ea260, aPoint=@0xbfffec88, aEvent=0xbfffee34, aCaptured=0) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1390
#16 0x0c406f15 in nsViewManager::DispatchEvent (this=0x2e0ea200, aEvent=0xbfffee34, aStatus=0xbfffed18) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1349
#17 0x0c3fd4e9 in HandleEvent (aEvent=0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsView.cpp:167
#18 0x271b3024 in nsChildView::DispatchEvent (this=0x2ef69960, event=0xbfffee34, aStatus=@0xbfffedec) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1791
#19 0x271a5a36 in nsChildView::DispatchWindowEvent (this=0x2ef69960, event=@0xbfffee34) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1804
#20 0x271b476a in -[ChildView mouseDragged:] (self=0x2ef69a00, _cmd=0x90ab7aac, theEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3341
#21 0x93375625 in -[NSWindow sendEvent:] ()
#22 0x271a4a71 in -[NSWindow(MethodSwizzling) nsCocoaWindow_NSWindow_sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:2201
#23 0x2719c2e7 in -[ToolbarWindow sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x3872efc0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:1895
#24 0x93366d8c in -[NSApplication sendEvent:] ()
#25 0x932918e7 in -[NSApplication run] ()
#26 0x27197bf5 in nsAppShell::Run (this=0x22294e0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#27 0x2a0f50f7 in nsAppStartup::Run (this=0x22458d0) at /Users/starapica/Documents/mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#28 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at /Users/starapica/Documents/mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#29 0x000027a4 in main (argc=5, argv=0xbffff9a4) at /Users/starapica/Documents/mozilla/HG/src/browser/app/nsBrowserApp.cpp:156
Leaving GenerateDragGesture

But when nsEventStateManager::GenerateDragGesture is invoked from mouseMoved nsBaseDragService::EndDragSession is not invoked (not sure why):

#0  nsBaseDragService::StartDragSession (this=0x34509760) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:334
#1  0x271946df in nsDragService::InvokeDragSession (this=0x34509760, aDOMNode=0x2e0fbf3c, aTransferableArray=0x3874bc80, aDragRgn=0x0, aActionType=7) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:301
#2  0x271e03e7 in nsBaseDragService::InvokeDragSessionWithImage (this=0x34509760, aDOMNode=0x2e0fbf3c, aTransferableArray=0x3874bc80, aRegion=0x0, aActionType=7, aImage=0x0, aImageX=0, aImageY=0, aDragEvent=0x2ef08680, aDataTransfer=0x387a71c0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:275
#3  0x0c2a2463 in nsEventStateManager::DoDefaultDragStart (this=0x2e0f3fb0, aPresContext=0x2c61a00, aDragEvent=0xbfffe430, aDataTransfer=0x387a71c0, aDragTarget=0x2e0fbf20, aIsSelection=0) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2329
#4  0x0c2a2c11 in nsEventStateManager::GenerateDragGesture (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffecb0) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2084
#5  0x0c2a9407 in nsEventStateManager::PreHandleEvent (this=0x2e0f3fb0, aPresContext=0x2c61a00, aEvent=0xbfffecb0, aTargetFrame=0x2df8438, aStatus=0xbfffea28, aView=0x2e0ea260) at /Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:859
#6  0x0bfeccb3 in PresShell::HandleEventInternal (this=0x2b42800, aEvent=0xbfffecb0, aView=0x2e0ea260, aStatus=0xbfffea28) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5915
#7  0x0bfed17d in PresShell::HandlePositionedEvent (this=0x2b42800, aView=0x2e0ea260, aTargetFrame=0x2df8438, aEvent=0xbfffecb0, aEventStatus=0xbfffea28) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#8  0x0bfef1fb in PresShell::HandleEvent (this=0x2b42800, aView=0x2e0ea260, aEvent=0xbfffecb0, aEventStatus=0xbfffea28) at /Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#9  0x0c405fda in nsViewManager::HandleEvent (this=0x2e0ea200, aView=0x2e0ea260, aPoint=@0xbfffeb08, aEvent=0xbfffecb0, aCaptured=0) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1390
#10 0x0c406f15 in nsViewManager::DispatchEvent (this=0x2e0ea200, aEvent=0xbfffecb0, aStatus=0xbfffeb98) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1349
#11 0x0c3fd4e9 in HandleEvent (aEvent=0xbfffecb0) at /Users/starapica/Documents/mozilla/HG/src/view/src/nsView.cpp:167
#12 0x271b3024 in nsChildView::DispatchEvent (this=0x2ef69960, event=0xbfffecb0, aStatus=@0xbfffec6c) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1791
#13 0x271a5a36 in nsChildView::DispatchWindowEvent (this=0x2ef69960, event=@0xbfffecb0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1804
#14 0x271b450a in -[ChildView mouseMoved:] (self=0x2ef69a00, _cmd=0x90ab7be4, theEvent=0x38776820) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3304
#15 0x271b41f9 in -[ChildView mouseMoved:] (self=0x2e0f6e60, _cmd=0x90ab7be4, theEvent=0x38776820) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3234
#16 0x93375625 in -[NSWindow sendEvent:] ()
#17 0x271a4a71 in -[NSWindow(MethodSwizzling) nsCocoaWindow_NSWindow_sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x38776820) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:2201
#18 0x2719c2e7 in -[ToolbarWindow sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4, anEvent=0x38776820) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:1895
#19 0x93366d8c in -[NSApplication sendEvent:] ()
#20 0x932918e7 in -[NSApplication run] ()
#21 0x27197bf5 in nsAppShell::Run (this=0x22294e0) at /Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#22 0x2a0f50f7 in nsAppStartup::Run (this=0x22458d0) at /Users/starapica/Documents/mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#23 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at /Users/starapica/Documents/mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#24 0x000027a4 in main (argc=5, argv=0xbffff9a4) at /Users/starapica/Documents/mozilla/HG/src/browser/app/nsBrowserApp.cpp:156


So, I added call to nsBaseDragService::EndDragSession() after return from mNativeDragView when drag is still invoked. This is more a workaround then a true solution, but seems it doesn't break anything and is quit straight forward solution.

To really fix this, should we remove invocation of GenerateDragGesture from mouseMoved?
Attachment #343288 - Attachment is obsolete: true
Attachment #343391 - Flags: review?(roc)
What exactly is going on in the first case? It looks like we started dragging, and then we started another drag in the middle which cancels the drag in progress? That sounds weird.
If the first part of your patch helps with this bug, we should check that part in and probably file a new bug about the other issue you found while testing.
(In reply to comment #11)
> What exactly is going on in the first case? It looks like we started dragging,
> and then we started another drag in the middle which cancels the drag in
> progress? That sounds weird.

I had to explain more in detail the backtraces. The first backtrace titled as 'correct drag' consist of two backtraces. It is outline of where from StartDragSession and EndDragSession is called. Bellow nsDragService::InvokeDragSession frame the two backtraces are identical (it is the same call of that method!). Correctly explained:

- Invocation of StartDragSession:

#0  nsBaseDragService::StartDragSession (this=0x34509760) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:334
#1  0x271946df in nsDragService::InvokeDragSession (this=0x34509760,
aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aDragRgn=0x0,
aActionType=7) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:301

- Invocation of EndDragSession (called from inside of NSDrag):

#0  nsBaseDragService::EndDragSession (this=0x34509760, aDoneDrag=1) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:342
#1  0x27191dff in nsDragService::EndDragSession (this=0x34509760, aDoneDrag=1)
at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:579
#2  0x271ac92b in -[ChildView draggedImage:endedAt:operation:]
(self=0x2ef69a00, _cmd=0x90aa6314, anImage=0x3874be10, aPoint={x = 906, y =
424}, operation=0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:5914
#3  0x9357ee4a in -[NSFilePromiseDragSource draggedImage:endedAt:operation:] ()
#4  0x9354db35 in -[NSCoreDragManager _dragUntilMouseUp:accepted:] ()
#5  0x9354cee7 in -[NSCoreDragManager
dragImage:fromWindow:at:offset:event:pasteboard:source:slideBack:] ()
#6  0x9354c93f in -[NSWindow(NSDrag)
dragImage:at:offset:event:pasteboard:source:slideBack:] ()
#7  0x27194830 in nsDragService::InvokeDragSession (this=0x34509760,
aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0, aDragRgn=0x0,
aActionType=7) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsDragService.mm:313

- Both backtraces origin from here (handle of a single same event):

#8  0x271e03e7 in nsBaseDragService::InvokeDragSessionWithImage
(this=0x34509760, aDOMNode=0x2ef6d92c, aTransferableArray=0x387233b0,
aRegion=0x0, aActionType=7, aImage=0x0, aImageX=0, aImageY=0,
aDragEvent=0x387aab30, aDataTransfer=0x2e07c1b0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/xpwidgets/nsBaseDragService.cpp:275
#9  0x0c2a2463 in nsEventStateManager::DoDefaultDragStart (this=0x2e0f3fb0,
aPresContext=0x2c61a00, aDragEvent=0xbfffe5b0, aDataTransfer=0x2e07c1b0,
aDragTarget=0x2ef6d910, aIsSelection=0) at
/Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2329
#10 0x0c2a2c11 in nsEventStateManager::GenerateDragGesture (this=0x2e0f3fb0,
aPresContext=0x2c61a00, aEvent=0xbfffee34) at
/Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:2084
#11 0x0c2a9407 in nsEventStateManager::PreHandleEvent (this=0x2e0f3fb0,
aPresContext=0x2c61a00, aEvent=0xbfffee34, aTargetFrame=0x2df8274,
aStatus=0xbfffeba8, aView=0x2e0ea260) at
/Users/starapica/Documents/mozilla/HG/src/content/events/src/nsEventStateManager.cpp:859
#12 0x0bfeccb3 in PresShell::HandleEventInternal (this=0x2b42800,
aEvent=0xbfffee34, aView=0x2e0ea260, aStatus=0xbfffeba8) at
/Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5915
#13 0x0bfed17d in PresShell::HandlePositionedEvent (this=0x2b42800,
aView=0x2e0ea260, aTargetFrame=0x2df8274, aEvent=0xbfffee34,
aEventStatus=0xbfffeba8) at
/Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5813
#14 0x0bfef1fb in PresShell::HandleEvent (this=0x2b42800, aView=0x2e0ea260,
aEvent=0xbfffee34, aEventStatus=0xbfffeba8) at
/Users/starapica/Documents/mozilla/HG/src/layout/base/nsPresShell.cpp:5673
#15 0x0c405fda in nsViewManager::HandleEvent (this=0x2e0ea200,
aView=0x2e0ea260, aPoint=@0xbfffec88, aEvent=0xbfffee34, aCaptured=0) at
/Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1390
#16 0x0c406f15 in nsViewManager::DispatchEvent (this=0x2e0ea200,
aEvent=0xbfffee34, aStatus=0xbfffed18) at
/Users/starapica/Documents/mozilla/HG/src/view/src/nsViewManager.cpp:1349
#17 0x0c3fd4e9 in HandleEvent (aEvent=0xbfffee34) at
/Users/starapica/Documents/mozilla/HG/src/view/src/nsView.cpp:167
#18 0x271b3024 in nsChildView::DispatchEvent (this=0x2ef69960,
event=0xbfffee34, aStatus=@0xbfffedec) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1791
#19 0x271a5a36 in nsChildView::DispatchWindowEvent (this=0x2ef69960,
event=@0xbfffee34) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:1804
#20 0x271b476a in -[ChildView mouseDragged:] (self=0x2ef69a00, _cmd=0x90ab7aac,
theEvent=0x3872efc0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsChildView.mm:3341
#21 0x93375625 in -[NSWindow sendEvent:] ()
#22 0x271a4a71 in -[NSWindow(MethodSwizzling)
nsCocoaWindow_NSWindow_sendEvent:] (self=0x2b7bf020, _cmd=0x90ac34c4,
anEvent=0x3872efc0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:2201
#23 0x2719c2e7 in -[ToolbarWindow sendEvent:] (self=0x2b7bf020,
_cmd=0x90ac34c4, anEvent=0x3872efc0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsCocoaWindow.mm:1895
#24 0x93366d8c in -[NSApplication sendEvent:] ()
#25 0x932918e7 in -[NSApplication run] ()
#26 0x27197bf5 in nsAppShell::Run (this=0x22294e0) at
/Users/starapica/Documents/mozilla/HG/src/widget/src/cocoa/nsAppShell.mm:591
#27 0x2a0f50f7 in nsAppStartup::Run (this=0x22458d0) at
/Users/starapica/Documents/mozilla/HG/src/toolkit/components/startup/src/nsAppStartup.cpp:182
#28 0x002109ef in XRE_main (argc=5, argv=0xbffff9a4, aAppData=0x220bf30) at
/Users/starapica/Documents/mozilla/HG/src/toolkit/xre/nsAppRunner.cpp:3263
#29 0x000027a4 in main (argc=5, argv=0xbffff9a4) at
/Users/starapica/Documents/mozilla/HG/src/browser/app/nsBrowserApp.cpp:156


The second backtrace shows where from StartDragSession is invoked for which complement EndDragSession is missing - not originating from 'mouseDrag' but from 'mouseMove'.
 
(In reply to comment #12)
> If the first part of your patch helps with this bug, we should check that part
> in and probably file a new bug about the other issue you found while testing.

I'm not sure the first part helps. When I normally use the browser (or at least use some little effort) I reproduce the bug after short amount of time w/ or w/o the first part of the patch. So, the second part is IMHO more important then the first one.
Okay, thanks for clearing that up a bit.

I guess when we're receiving a Cocoa mouseMoved event, and we interpret it as a drag and try to call InvokeDragSession which calls [mNativeDragView dragImage], Cocoa thinks we're not really dragging and doesn't call draggedImage. So in some sense dragImage only really "works" when triggered by a Cocoa mouseDragged event. Stephen, Josh, does that sound right?
(In reply to comment #14)

Sounds reasonable to me.  But to be really sure what's going on, I'd
need to play with this stuff for a while myself.  Can I put that off
til next week?  (Probably Tuesday or Wednesday.)
(In reply to comment #14)So in
> some sense dragImage only really "works" when triggered by a Cocoa mouseDragged
> event.

That's correct. dragImage can only be called during mouseDown or mouseDragged. The Cocoa documentation clearly states this.
Steven, STR to reproduce the secondary problem:
- apply the first part of the patch (mouse coord update fix)
- http://www.vesmir.cz/clanek.php3?CID=7876
- drag (in south direction) very quickly in this order: the blue banner, the
top title "vesmír", the top-left link to a book "vesmír", one of the yellow
links from the section menu
- the last drag should not invoke EndDragSession call i.e. should be based just
on 'mouseMove' event

Hope it helps.
(In reply to comment #16)
> That's correct. dragImage can only be called during mouseDown or mouseDragged.
> The Cocoa documentation clearly states this.

And to avoid regression, shouldn't StartDragSession also be somehow called from [mNativeDragView dragImage]?
(In reply to comment #18)
> (In reply to comment #16)
> > That's correct. dragImage can only be called during mouseDown or mouseDragged.
> > The Cocoa documentation clearly states this.
> 
> And to avoid regression, shouldn't StartDragSession also be somehow called from
> [mNativeDragView dragImage]?

It's already called just before dragImage is called.
(In reply to comment #19)
> > And to avoid regression, shouldn't StartDragSession also be somehow called from
> > [mNativeDragView dragImage]?
> 
> It's already called just before dragImage is called.

And that is the problem. Because dragImage is responsible for calling complementary EndDragSession to StartDragSession but fails to call it from whatever reason, the flag indicating drag is in progress hangs forever => one of causes to this bug.
OK then, the question is why we're doing a StartDragSession under a mouseMoved. It seems to me that we must have had a StartTrackingDragGesture but then the mouse button was physically released (which Cocoa knows about) but we didn't see the mouse-up event for some reason, so StopTrackingDragGesture in nsEventStateManager::PreHandleEvent for NS_MOUSE_BUTTON_UP was not called.

I think in nsEventStateManager::GenerateDragGesture, we should check if the mouse button is up and if so call StopTrackingDragGesture and exit, just in case we missed the mouse-up event.
Attached patch Just a debugging patch (obsolete) — Splinter Review
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_UP
*** StopTrackingDragGesture
*** MOUSE_MOVE
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_UP
*** StopTrackingDragGesture
*** MOUSE_MOVE
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
> ***** Bad invoke!!!
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_UP
*** StopTrackingDragGesture
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_MOVE
*** MOUSE_DOWN

This is short a log where's recorded sequence of method calls and the 'bad situation' - endDragSession was not called. I enclose a patch with these printfs added, better then to explain. You can see there are two MOUSE_DOWN events. This is pure guess but I would say we mess up somehow the source object reference and imageDrag gets creazy from it, probably in some of our callbacks.

Any idea?
Actually, interesting is just this part, between forst MOUSE_DOWN and opposite MOUSE_UP:

*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
> ***** Bad invoke!!!
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
*** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** Leaving GenerateDragGesture
*** MOUSE_UP
What's not clear from your log is which MOUSE_MOVEs are generated by Cocoa mouseDragged events and which are generated by Cocoa mouseMoved events. That would be useful to know.

Also, I don't know why we're getting consecutive MOUSE_DOWNs without an intervening MOUSE_UP. That shouldn't have anything to do with how the events are targeted, unless somehow a MOUSE_UP is occurring outside the browser window and we failed to capture it.

Anyway, I think we should do the fix in comment #21 whether or not we understand why we're missing MOUSE_UP events.
You are right. When dragImage is invoked from 'mouseMove' and not from 'mouseDrag' we get into the bad state. All correct drags are invoked from 'mouseDrag' event. It is normal, as I observe the log, to miss MOUSE_UP and it doesn't cause the original problem.

This is log of two drags performed immedietly each after other, the second one is our 'bad state':

*** MOUSE_DOWN
*** BeginTrackingDragGesture
- syntheticMouseMove *** MOUSE_MOVE
*** Entering GenerateDragGesture
*** Leaving GenerateDragGesture
- mouseDragged *** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** invoking dragImage
*** Leaving GenerateDragGesture
*** MOUSE_DOWN
*** BeginTrackingDragGesture
- mouseMoved *** MOUSE_MOVE
*** Entering GenerateDragGesture
*** StopTrackingDragGesture
*** invoking dragImage
>***** Bad invoke!!!
*** Leaving GenerateDragGesture

'Entering GenerateDragGesture' and immediate 'Leaving GenerateDragGesture' pair means we didn't pass the threshold for drag invoke.

I agree we should land the fix v2 and open a followup for this one issue.
Attachment #344167 - Attachment is obsolete: true
For reference if someone interested...
Honza - can you post a minimal patch that we can land now and we'll file a bug on the other issue you found? I think roc is out for a bit but I'll review and I bet bz will sr.
I believe attachment 343391 [details] [diff] [review] is the patch we want. It's first part is already reviewed and tested fix for mouse coords update and the second part is a safe-workaround to drop running drag session when it failed to end within dragImage call, from whatever reason; it just prevents d&d lock up, doesn't fix the problematic drag. We have to find the reason and true fix for it in a follow up.
Comment on attachment 343391 [details] [diff] [review]
fix, v2

r? -> josh
Attachment #343391 - Flags: review?(roc) → review?(joshmoz)
Attachment #343391 - Flags: review?(joshmoz) → review+
Attachment #343391 - Flags: superreview?(bzbarsky)
Blocks: 461207
Comment on attachment 343391 [details] [diff] [review]
fix, v2

sr=bzbarsky
Attachment #343391 - Flags: superreview?(bzbarsky) → superreview+
please check-in comment 10
Keywords: checkin-needed
landed on trunk
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
This caused 
MacOSX Darwin 9.2.2 moz2-darwin8-slave01 dep unit test on 2008/10/23 01:53:22
*** 67709 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_colorpicker_popup.xul | key up while open - got "#FFFFFF", expected "#FF6666"
*** 67712 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_colorpicker_popup.xul | key down while open - got "#CCCCCC", expected "#FF0000"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The test failure reason:
- there is simulation of a mouse click (mouse down, mouse up pair) on point 2,2 of the colorpicker widget, this opens and closes the color picker
- fix for this bug updates the mouse coordintes during this simulated click to the global screen point
- at some moment the test forces the whole window to scroll a bit down and up and this produces a synthetic mouse move event that is fired at the same point of the SCREEN where the simulated click was invoked
- at that moment there is at that point (because of the scroll) top-left square (the white color) of the color picker pane => this changes the focused color

Nail or anyone, please help me to deicide on solution:
1. we can find cause of the scroll (that should not be invoked IMHO) and fix it; might take a long time and I want to get fix for this bug to ff3.1b1 (it is blocking to it)
2. run the test in a separate window that would not allow scroll of the whole page (a quick work around, the test passes that way and is still valid)
Blocks: 387097
Do you see this scroll when running the test without your patch? I don't see it.
(In reply to comment #36)
> Do you see this scroll when running the test without your patch? I don't see
> it.

I see it. Are you sure it is not visible when you remove it? One thing is different anyway. There is assertion failing during the scroll when my patch is applied. I'll investigate it.
I'm running a trunk build; when I run that mochitest, I don't see any scrolling. In fact the test page is small so there are no scrollbars.
You have to run all tests on the page ("Run Tests" link) and you will see it. Also I can reproduce it only when the window is zoomed to full size. There is following assertion during the scroll, appearing only with the patch applied; but might be a wrong way:

###!!! ASSERTION: cannot call GetUsedBorder on a dirty frame not currently being reflowed: 'nsLayoutUtils::sDisableGetUsedXAssertions || !NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file /Users/starapica/Documents/mozilla/HG/src/layout/generic/nsFrame.cpp, line 608
Attachment #343391 - Attachment is obsolete: true
Attachment #344994 - Flags: review?(roc)
Test has been workaround'ed, see bug 461857.
Keywords: checkin-needed
Comment on attachment 344994 [details] [diff] [review]
fix, v2 + colorpicker test update
[Checkin: Comment 42]

http://hg.mozilla.org/mozilla-central/rev/cb0075c50854
Attachment #344994 - Attachment description: fix, v2 + colorpicker test update → fix, v2 + colorpicker test update [Checkin: Comment 42]
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Version: unspecified → Trunk
(In reply to comment #41)
> Test has been workaround'ed, see bug 461857.

Are you planning on fixing that bug in the near future?

I don't like that the patch here changed an unrelated test to 'fix' a failure introduced by this bug. The colorpicker scrolling may be a bug to fix as well, but it seems like this patch has changed some behaviour related to events that is still present and could affect behaviour that the tests do not test. Or am I just not seeing the description of an intended change in the comments above?
(In reply to comment #43)
> Are you planning on fixing that bug in the near future?

I could work on it after I fix all blockings for ff3.1b2. I don't think there is need to do additional testing if I understand your concern.
Comment #35 (In reply to comment #43)
> Are you planning on fixing that bug in the near future?

I don't think Honza is responsible for fixing the color picker. Anyway, Martijn has a patch in bug 426590.

> I don't like that the patch here changed an unrelated test to 'fix' a failure
> introduced by this bug. The colorpicker scrolling may be a bug to fix as well,
> but it seems like this patch has changed some behaviour related to events that
> is still present and could affect behaviour that the tests do not test. Or am I
> just not seeing the description of an intended change in the comments above?

See comment #35 for the explanation for why this patch changed that test's behaviour. The new behaviour is correct. The old behaviour was that the synthetic mouse move dispatched on scrolling would sometimes have the wrong coordinates. The test was accidentally relying on that.
(In reply to comment #5)
> STR:
> 1. http://en-us.www.mozilla.com/en-US/firefox/central/
> 2. drag "mozilla" link from above (any of them) on the "Work" link (or any
> other) on one of the tabs
> 3. release mouse button and _do not move the mouse_
> 4. now press the mouse button again and try to drag "Work" link, it won't work
> from that moment
> 
> continuing with the code analyzes now.

Using STR here, verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090122 Minefield/3.2a1pre 
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090122 Shiretoko/3.1b3pre
Status: RESOLVED → VERIFIED
Depends on: 507572
Depends on: 507006
Depends on: 584873
You need to log in before you can comment on or make changes to this bug.