Closed Bug 548185 Opened 14 years ago Closed 14 years ago

Crash on event.dataTransfer.setDragImage({},0,0) [@ nsINode::GetCurrentDoc() ] [@ PresShell::RenderNode(nsIDOMNode*, nsIRegion*, nsIntPoint&, nsIntRect*) ]

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking1.9.2 --- -
status1.9.2 --- .4-fixed
blocking1.9.1 --- -
status1.9.1 --- .10-fixed

People

(Reporter: davemgarrett, Assigned: smaug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [sg:dos])

Crash Data

Attachments

(3 files, 1 obsolete file)

Attached file testcase
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.

bp-8e5d0035-68b5-49c6-aa83-c00802100223

bp-b69b8122-33c1-49b9-ad8b-490142100223
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
By the way, passing null or undefined instead of {} does not crash.
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.
blocking1.9.1: ? → -
blocking1.9.2: ? → -
Keywords: crash
Whiteboard: [sg:dos]
Keywords: testcase
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #429392 - Flags: review?(Olli.Pettay)
Attachment #429392 - Flags: review?(Olli.Pettay) → review+
http://hg.mozilla.org/mozilla-central/rev/8fdae602e9b0
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Version: 1.9.1 Branch → Trunk
Attachment #429392 - Flags: approval1.9.2.2?
Attachment #429392 - Flags: approval1.9.1.9?
Attachment #429392 - Flags: approval1.9.2.2?
Attachment #429392 - Flags: approval1.9.2.2+
Attachment #429392 - Flags: approval1.9.1.9?
Attachment #429392 - Flags: approval1.9.1.9+
Comment on attachment 429392 [details] [diff] [review]
not all nsIDOMNodes are nsIContents (and all scriptable emulating objects are not)

a1922,1919=beltzner
Well, it probably wouldn't hurt to get it onto there too if anyone wanted to.
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 http://crash-stats.mozilla.com/report/index/418b0311-0a16-493a-a893-7630f2100322 for 3.6.2.

See http://crash-stats.mozilla.com/report/index/bp-8480fb00-043c-4ca9-bf24-e9b9d2100322 for 3.5.9.

Both tested with clean profiles on a clean Windows XP virtual machine.
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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Crash on event.dataTransfer.setDragImage({},0,0) [@ nsINode::GetCurrentDoc() ] → Crash on event.dataTransfer.setDragImage({},0,0) [@ nsINode::GetCurrentDoc() ] [@ PresShell::RenderNode(nsIDOMNode*, nsIRegion*, nsIntPoint&, nsIntRect*) ]
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/mozilla.org/mozilla-central/layout/base/nsPresShell.cpp:5486
5486	  if (!node->IsInDoc())
(gdb) l
5481	  nsRect area;
5482	  nsTArray<nsAutoPtr<RangePaintInfo> > rangeItems;
5483	
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.
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.
Attachment #429392 - Attachment is obsolete: true
Attachment #434017 - Flags: review?(Olli.Pettay)
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.
Attachment #429392 - Attachment is obsolete: false
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.
Attachment #434017 - Flags: review?(Olli.Pettay) → review-
Attached patch patchSplinter Review
This way we don't need to patch all the cases when drag code
QI's nsIDOMElement to nsINode
Attachment #434564 - Flags: review?(enndeakin)
Attachment #434564 - Flags: review?(enndeakin) → review+
Assignee: timeless → Olli.Pettay
http://hg.mozilla.org/mozilla-central/rev/be976867b094
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Are you going to want to land this follow-up patch on the same branches as the first patch?
Comment on attachment 434564 [details] [diff] [review]
patch

we'd want them, but presumably he'd need approval.
Attachment #434564 - Flags: approval1.9.2.3?
Attachment #434564 - Flags: approval1.9.1.10?
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: https://bug548185.bugzilla.mozilla.org/attachment.cgi?id=428615
Line: 6

Looks good now.
Attachment #434017 - Attachment is obsolete: true
Comment on attachment 434564 [details] [diff] [review]
patch

a=beltzner for 1.9.2.3 and 1.9.1.10
Attachment #434564 - Flags: approval1.9.2.3?
Attachment #434564 - Flags: approval1.9.2.3+
Attachment #434564 - Flags: approval1.9.1.10?
Attachment #434564 - Flags: approval1.9.1.10+
Crash Signature: [@ nsINode::GetCurrentDoc() ] [@ PresShell::RenderNode(nsIDOMNode*, nsIRegion*, nsIntPoint&, nsIntRect*) ]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: