[webkit-reviews] review requested: [Bug 57731] [CSS 3] missing cursor support for 'none' on Windows : [Attachment 104642] Revised patch addressing comments.

[bugzilla-daemon at webkit.org]

Ryan <aikavanak at gmail.com> has asked  for review:
Bug 57731: [CSS 3] missing cursor support for 'none' on Windows
https://bugs.webkit.org/show_bug.cgi?id=57731

Attachment 104642: Revised patch addressing comments.
https://bugs.webkit.org/attachment.cgi?id=104642&action=review

------- Additional Comments from Ryan <aikavanak at gmail.com>
Updated patch to address comments. It was simple to add an ASSERT for WebKit,
but I ended up not doing an ASSERT for WebKit2 for the following reasons:
1. Not asserting and just returning without setting the cursor was closer to
how the code worked before this patch, so it seems less risky.
2. I noticed that WebView::cursorToShow returns 0 if the page isn't valid,
which may be the case even if the WebCore cursor's Type isn't None. I figured
it would be better to continue with DefWindowProc's cursor handling rather than
asserting.

If these are bad reasons, I'll be happy to update the patch.

With this method of implementing cursor: none, the convention of a null
override cursor meaning "don't use an override" won't work if the override
cursor itself ever needs to be 'none'. Is this a case that we need to handle?

The flickering I was seeing was happening because in some cases the
DefWindowProc was being called even though the WM_SETCURSOR message was being
handled. DefWindowProc handles WM_SETCURSOR by setting the cursor to the window
class' cursor, which for the WebView is an arrow. Since the WebView always
calls ::SetCursor when it gets a WM_SETCURSOR message, it should never need to
call DefWindowProc in this case.