Bug 45692 – REGRESSION (r63339): Mouse cursor disappears when holding mouse button down on page

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

To reproduce:

1. Hold the left mouse button down on any web page

The mouse cursor disappears. This makes it hard to drag to select text or drag the scrollbar thumb.

[Adam Roben (:aroben)]

<rdar://problem/8423469>

[Adam Roben (:aroben)]

r63339 seems awfully suspect: <http://trac.webkit.org/changeset/63339>

[Adam Roben (:aroben)]

The docs for SetCursor say <http://msdn.microsoft.com/en-us/library/ms648393(v=vs.85).aspx>:

> If this parameter is NULL, the cursor is removed from the screen.

That seems pretty likely. :-)

[Adam Roben (:aroben)]

I think the problem is that WebView::m_lastSetCursor isn't getting set when the UI delegate implements webViewSetCursor.

[Adam Roben (:aroben)]

For posterity:

If you want to set a breakpoint when someone calls SetCusor(0), you can do it like this:

1. Add a new function breakpoint for {,,user32.dll}_NtUserSetFocus@4
2. Set the condition on the breakpoint to be: *(int*)(esp+4) == 0

[Adam Roben (:aroben)]

Created attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us

[Brian Weinstein]

Comment on attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us

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

> WebKit/win/WebCoreSupport/WebChromeClient.cpp:810
> +        ::SetCursor(platformCursor);

Is it useful to assert that the cursor is non-null here?

> WebKit/win/WebCoreSupport/WebChromeClient.cpp:812
> +    setLastSetCursorToCurrentCursor();

Is there a reason this wasn't called before/was it called from somewhere else before this patch?

[Adam Roben (:aroben)]

Comment on attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us

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

>> WebKit/win/WebCoreSupport/WebChromeClient.cpp:810
>> +        ::SetCursor(platformCursor);
> 
> Is it useful to assert that the cursor is non-null here?

I don't think so. We already null-checked platformCursor on line 797 above.

>> WebKit/win/WebCoreSupport/WebChromeClient.cpp:812
>> +    setLastSetCursorToCurrentCursor();
> 
> Is there a reason this wasn't called before/was it called from somewhere else before this patch?

Prior to this patch, m_webView->setLastCursor was called. setLastSetCursorToCurrentCursor() calls that same function, but passes it the result of ::GetCursor(), which should now return the cursor we (or the UI delegate) just passed to ::SetCursor.

[Ada Chan]

Comment on attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us

r=me

[Adam Roben (:aroben)]

Committed r74055: <http://trac.webkit.org/changeset/74055>

[Adam Roben (:aroben)]

(In reply to comment #5)
> 1. Add a new function breakpoint for {,,user32.dll}_NtUserSetFocus@4

That should be {,,user32.dll}_NTUserSetCursor@4