Bug 45692 - REGRESSION (r63339): Mouse cursor disappears when holding mouse button down on page
Summary: REGRESSION (r63339): Mouse cursor disappears when holding mouse button down o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar, PlatformOnly, Regression
Depends on:
Blocks:
 
Reported: 2010-09-13 11:33 PDT by Adam Roben (:aroben)
Modified: 2011-01-06 14:43 PST (History)
1 user (show)

See Also:


Attachments
Always record the last-set cursor, even when the UI delegate is setting the cursor for us (2.27 KB, patch)
2010-12-14 12:20 PST, Adam Roben (:aroben)
adachan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2010-09-13 11:33:36 PDT
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.
Comment 1 Adam Roben (:aroben) 2010-09-13 11:35:05 PDT
<rdar://problem/8423469>
Comment 2 Adam Roben (:aroben) 2010-09-15 08:17:15 PDT
r63339 seems awfully suspect: <http://trac.webkit.org/changeset/63339>
Comment 3 Adam Roben (:aroben) 2010-12-14 11:08:13 PST
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. :-)
Comment 4 Adam Roben (:aroben) 2010-12-14 11:51:16 PST
I think the problem is that WebView::m_lastSetCursor isn't getting set when the UI delegate implements webViewSetCursor.
Comment 5 Adam Roben (:aroben) 2010-12-14 11:54:21 PST
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
Comment 6 Adam Roben (:aroben) 2010-12-14 12:20:36 PST
Created attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us
Comment 7 Brian Weinstein 2010-12-14 12:29:27 PST
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?
Comment 8 Adam Roben (:aroben) 2010-12-14 12:45:42 PST
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.
Comment 9 Ada Chan 2010-12-14 12:58:19 PST
Comment on attachment 76556 [details]
Always record the last-set cursor, even when the UI delegate is setting the cursor for us

r=me
Comment 10 Adam Roben (:aroben) 2010-12-14 13:08:04 PST
Committed r74055: <http://trac.webkit.org/changeset/74055>
Comment 11 Adam Roben (:aroben) 2011-01-06 14:43:52 PST
(In reply to comment #5)
> 1. Add a new function breakpoint for {,,user32.dll}_NtUserSetFocus@4

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