Bug 57732

Summary: [CSS 3] setting the 'cursor' property to 'no-drop' causes a crash
Product: WebKit Reporter: Ryan <aikavanak>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, aroben, jeffm
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Minimal test based on manual-tests/cursor.html
none
Proposed patch
eric: review-
Backtrace none

Description Ryan 2011-04-03 14:26:19 PDT
Created attachment 88021 [details]
Minimal test based on manual-tests/cursor.html

When the cursor is set to 'no-drop', WebKit will crash. Appears to be because no platform cursor is set in the Cursor::NoDrop case in Cursor::ensurePlatformCursor() in CursorWin.cpp.
Comment 1 Ryan 2011-04-03 22:33:59 PDT
Created attachment 88033 [details]
Proposed patch

On Windows, set a cursor for the 'no-drop' property, instead of leaving it unset and crashing. The cursor is the same one used for 'not-allowed' (IDC_NO).
Comment 2 Adam Roben (:aroben) 2011-04-04 11:29:33 PDT
Comment on attachment 88033 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=88033&action=review

> Source/WebCore/ChangeLog:11
> +        No new tests. WebCore/manual-tests/cursor.html already contains a
> +        test for 'no-drop'.

Is it possible to make an automated test that would crash without this fix?
Comment 3 Adam Roben (:aroben) 2011-04-04 11:29:50 PDT
It would be nice to have a backtrace for the crash for future reference.
Comment 4 Ryan 2011-04-07 00:40:33 PDT
Created attachment 88587 [details]
Backtrace

Backtrace from Visual Studio 2005 Express debugger. Occurred when I moved the mouse cursor over the "nodrop" div in manual-tests/cursor.html.
Comment 5 Adam Roben (:aroben) 2011-04-07 05:57:27 PDT
Thanks, Ryan!
Comment 6 Eric Seidel (no email) 2011-04-26 16:11:06 PDT
Comment on attachment 88033 [details]
Proposed patch

How do we tes this?  This seems obviously right otherwise.
Comment 7 Ryan 2011-05-25 17:56:40 PDT
(In reply to comment #6)
> (From update of attachment 88033 [details])
> How do we tes this?  This seems obviously right otherwise.

I'm still new to WebKit development, so I'm not sure what the best way to test this would be, given that browsers don't currently have a way to control the mouse cursor.

Would it be viable to do a somewhat artificial test, perhaps by launching Safari maximized, load a page with the no-drop style applied to the entire body, and call SetCursorPos to move the cursor onto the page?
Comment 8 Ryan 2011-10-01 15:36:06 PDT
Looks like this was fixed by jeffm@apple.com in revision 95057.
Comment 9 Jeff Miller 2011-10-03 09:40:14 PDT
(In reply to comment #8)
> Looks like this was fixed by jeffm@apple.com in revision 95057.

Yes, I remember fixing this exact crash in <http://trac.webkit.org/changeset/95057>.