Bug 57732 – [CSS 3] setting the 'cursor' property to 'no-drop' causes a crash

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Ryan]

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.

[Ryan]

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).

[Adam Roben (:aroben)]

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?

[Adam Roben (:aroben)]

It would be nice to have a backtrace for the crash for future reference.

[Ryan]

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.

[Adam Roben (:aroben)]

Thanks, Ryan!

[Eric Seidel (no email)]

Comment on attachment 88033 [details]
Proposed patch

How do we tes this?  This seems obviously right otherwise.

[Ryan]

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

[Ryan]

Looks like this was fixed by [e-mail] in revision 95057.

[Jeff Miller]

(In reply to comment #8)
> Looks like this was fixed by [e-mail] in revision 95057.

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