Last Comment Bug 548185 - Crash on event.dataTransfer.setDragImage({},0,0) [@ nsINode::GetCurrentDoc() ] [@ PresShell::RenderNode(nsIDOMNode*, nsIRegion*, nsIntPoint&, nsIntRect*) ]
: Crash on event.dataTransfer.setDragImage({},0,0) [@ nsINode::GetCurrentDoc() ...
: crash, regression, testcase
Product: Core
Classification: Components
Component: Drag and Drop (show other bugs)
: Trunk
: x86 All
-- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
: Neil Deakin
Depends on:
  Show dependency treegraph
Reported: 2010-02-23 19:29 PST by Dave Garrett
Modified: 2011-06-13 10:01 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (437 bytes, text/html)
2010-02-23 19:29 PST, Dave Garrett
no flags Details
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not) (656 bytes, patch)
2010-02-28 02:51 PST, timeless
bugs: review+
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
Details | Diff | Splinter Review
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not) part 2/2 (701 bytes, patch)
2010-03-22 13:24 PDT, timeless
bugs: review-
Details | Diff | Splinter Review
patch (1.26 KB, patch)
2010-03-24 10:29 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
enndeakin: review+
mbeltzner: approval1.9.2.4+
mbeltzner: approval1.9.1.10+
Details | Diff | Splinter Review

Description User image Dave Garrett 2010-02-23 19:29:59 PST
Created attachment 428615 [details]

event.dataTransfer.setDragImage({},0,0) crashes under Firefox 3.5, 3.6, and latest Trunk, but not 3.0. See attached testcase.

I initially stumbled onto it from XUL but it crashes from HTML too and the element being dragged doesn't matter. 100% reproducible for me under both Linux and Windows XP.


0  	xul.dll  	nsINode::GetCurrentDoc  	 obj-firefox/dist/include/nsINode.h:378
1 	xul.dll 	GetPresShellForContent 	widget/src/xpwidgets/nsBaseDragService.cpp:404
2 	xul.dll 	nsBaseDragService::DrawDrag 	widget/src/xpwidgets/nsBaseDragService.cpp:436
3 	xul.dll 	nsDragService::CreateDragImage 	widget/src/windows/nsDragService.cpp:117
4 	xul.dll 	nsDragService::InvokeDragSession 	widget/src/windows/nsDragService.cpp:258
5 	xul.dll 	nsBaseDragService::InvokeDragSessionWithImage 	widget/src/xpwidgets/nsBaseDragService.cpp:281
6 	xul.dll 	nsEventStateManager::DoDefaultDragStart 	content/events/src/nsEventStateManager.cpp:2233
7 	xul.dll 	nsEventStateManager::GenerateDragGesture 	
8 	xul.dll 	nsEventStateManager::PreHandleEvent 	content/events/src/nsEventStateManager.cpp:1123
9 	xul.dll 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:6509
10 	xul.dll 	PresShell::HandlePositionedEvent 	layout/base/nsPresShell.cpp:6365
11 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:6228
12 	xul.dll 	nsViewManager::HandleEvent 	view/src/nsViewManager.cpp:1139
13 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1118
14 	xul.dll 	HandleEvent 	view/src/nsView.cpp:160
15 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:3044
16 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/src/windows/nsWindow.cpp:3067
17 	xul.dll 	nsWindow::DispatchMouseEvent 	widget/src/windows/nsWindow.cpp:3450
18 	xul.dll 	ChildWindow::DispatchMouseEvent 	widget/src/windows/nsWindow.cpp:7273
Comment 1 User image Dave Garrett 2010-02-24 01:49:37 PST
By the way, passing null or undefined instead of {} does not crash.
Comment 2 User image Daniel Veditz [:dveditz] 2010-02-24 13:24:38 PST
Looks like a null deref which doesn't "block" on the old branches unless it's a high frequency crash, but once there's a fix we could look at landing it on the old branches.
Comment 3 User image timeless 2010-02-28 02:51:02 PST
Created attachment 429392 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not)
Comment 5 User image Mike Beltzner [:beltzner, not reading bugmail] 2010-03-03 12:52:16 PST
Comment on attachment 429392 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not)

Comment 8 User image Dave Garrett 2010-03-05 04:21:48 PST
Well, it probably wouldn't hurt to get it onto there too if anyone wanted to.
Comment 9 User image Al Billings [:abillings] 2010-03-22 12:16:20 PDT
This isn't fixed in 1.9.2 or 1.9.1. If I use the attached testcase on Firefox 3.6.2, build 3 or Firefox 3.5.9, I crash.

See for 3.6.2.

See for 3.5.9.

Both tested with clean profiles on a clean Windows XP virtual machine.
Comment 10 User image Dave Garrett 2010-03-22 12:54:36 PDT
Trunk crashes too. This is a new sig though. I think this was fixed and may have regressed again? Not sure.

Does setDragImage have some automated tests this could be added to?
Comment 11 User image timeless 2010-03-22 13:03:50 PDT
oops, my fault, thanks al.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000018
0x00000001003a4788 in nsINode::IsInDoc (this=0x0) at nsINode.h:370
370	    return mParentPtrBits & PARENT_BIT_INDOCUMENT;
(gdb) up
#1  0x0000000100418923 in PresShell::RenderNode (this=0x1194b5fc0, aNode=0x120b92cf0, aRegion=0x0, aPoint=@0x7fff5fbfd3e0, aScreenRect=0x7fff5fbfd950) at /Users/timeless/devel/
5486	  if (!node->IsInDoc())
(gdb) l
5481	  nsRect area;
5482	  nsTArray<nsAutoPtr<RangePaintInfo> > rangeItems;
5484	  // nothing to draw if the node isn't in a document
5485	  nsCOMPtr<nsINode> node = do_QueryInterface(aNode);

we need an extra guard here, i'm testing locally (including dragging the link...)

5486	  if (!node->IsInDoc())
5487	    return nsnull;

dave: dunno, but yeah, it definitely needs to go into one.
Comment 12 User image timeless 2010-03-22 13:24:52 PDT
Created attachment 434017 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not) part 2/2

ok, with this, the testcase no longer crashes.

i'm sick and don't have the energy to deal with producing a testcase. hopefully dave or smaug can produce one. i'm essentially unavailable for 3 weeks (one for health, two for travel).

i can understand not wanting to push this to branches w/o the testcase, although as al has shown, testing can verify the fix.
Comment 13 User image Dave Garrett 2010-03-22 14:21:41 PDT
Comment on attachment 429392 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not)

The first patch isn't really obsolete. It did fix the first crash and landed and wasn't backed out.
Comment 14 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2010-03-24 10:27:37 PDT
Comment on attachment 434017 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not) part 2/2

I'd like to prevent the crash earlier in the code.
Comment 15 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2010-03-24 10:29:12 PDT
Created attachment 434564 [details] [diff] [review]

This way we don't need to patch all the cases when drag code
QI's nsIDOMElement to nsINode
Comment 16 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2010-03-25 13:51:29 PDT
Comment 17 User image Daniel Veditz [:dveditz] 2010-03-25 18:42:13 PDT
Are you going to want to land this follow-up patch on the same branches as the first patch?
Comment 18 User image timeless 2010-03-25 22:05:44 PDT
Comment on attachment 434564 [details] [diff] [review]

we'd want them, but presumably he'd need approval.
Comment 19 User image Dave Garrett 2010-03-26 17:11:17 PDT
Latest Trunk now gives the following in the Error Console instead of a crash:

Error: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDOMDataTransfer.setDragImage]
Source file:
Line: 6

Looks good now.
Comment 20 User image Mike Beltzner [:beltzner, not reading bugmail] 2010-03-29 10:56:37 PDT
Comment on attachment 434564 [details] [diff] [review]

a=beltzner for and

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