45692 2010-09-13 11:33:36 -0700 REGRESSION (r63339): Mouse cursor disappears when holding mouse button down on page 2011-01-06 14:43:52 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) PC Windows XP RESOLVED FIXED InRadar, PlatformOnly, Regression P2 Normal --- 1 aroben webkit-unassigned sam oldest_to_newest 278341 0 aroben 2010-09-13 11:33:36 -0700 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. 278343 1 aroben 2010-09-13 11:35:05 -0700 <rdar://problem/8423469> 279552 2 aroben 2010-09-15 08:17:15 -0700 r63339 seems awfully suspect: <http://trac.webkit.org/changeset/63339> 322052 3 aroben 2010-12-14 11:08:13 -0800 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. :-) 322083 4 aroben 2010-12-14 11:51:16 -0800 I think the problem is that WebView::m_lastSetCursor isn't getting set when the UI delegate implements webViewSetCursor. 322087 5 aroben 2010-12-14 11:54:21 -0800 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 322107 6 76556 aroben 2010-12-14 12:20:36 -0800 Created attachment 76556 Always record the last-set cursor, even when the UI delegate is setting the cursor for us 322116 7 76556 bweinstein 2010-12-14 12:29:27 -0800 Comment on attachment 76556 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? 322127 8 76556 aroben 2010-12-14 12:45:42 -0800 Comment on attachment 76556 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. 322138 9 76556 adachan 2010-12-14 12:58:19 -0800 Comment on attachment 76556 Always record the last-set cursor, even when the UI delegate is setting the cursor for us r=me 322145 10 aroben 2010-12-14 13:08:04 -0800 Committed r74055: <http://trac.webkit.org/changeset/74055> 330438 11 aroben 2011-01-06 14:43:52 -0800 (In reply to comment #5) > 1. Add a new function breakpoint for {,,user32.dll}_NtUserSetFocus@4 That should be {,,user32.dll}_NTUserSetCursor@4 76556 2010-12-14 12:20:36 -0800 2010-12-14 12:58:19 -0800 Always record the last-set cursor, even when the UI delegate is setting the cursor for us bug-45692-20101214152034.patch text/plain 2321 aroben ZGlmZiAtLWdpdCBhL1dlYktpdC93aW4vQ2hhbmdlTG9nIGIvV2ViS2l0L3dpbi9DaGFuZ2VMb2cK aW5kZXggZDRjZWI2N2IyNjgwZDMyOTQzOTc3Y2U3MjM4OGI5MTY3MmFhMTJmYi4uYTBiYTdmMTRh MjQ5YzgxOGNjZThiY2U5MDc4ODc0ZWQ2ZDUwMjU0YSAxMDA2NDQKLS0tIGEvV2ViS2l0L3dpbi9D aGFuZ2VMb2cKKysrIGIvV2ViS2l0L3dpbi9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyNCBAQAorMjAx MC0xMi0xNCAgQWRhbSBSb2JlbiAgPGFyb2JlbkBhcHBsZS5jb20+CisKKyAgICAgICAgQWx3YXlz IHJlY29yZCB0aGUgbGFzdC1zZXQgY3Vyc29yLCBldmVuIHdoZW4gdGhlIFVJIGRlbGVnYXRlIGlz CisgICAgICAgIHNldHRpbmcgdGhlIGN1cnNvciBmb3IgdXMKKworICAgICAgICBQcmlvciB0byBy NjMzMzksIHRoZSBsYXN0LXNldCBjdXJzb3Igd2FzIHJlY29yZGVkIGluIFdpZGdldDo6c2V0Q3Vy c29yLgorICAgICAgICByNjMzMzkgbW92ZWQgdGhhdCBjb2RlIHVwIHRvIFdlYkNocm9tZUNsaWVu dCwgYnV0IGZhaWxlZCB0byBjYWxsIGl0CisgICAgICAgIHdoZW4gdGhlIFVJIGRlbGVnYXRlIHdh cyB0aGUgb25lIHNldHRpbmcgdGhlIGN1cnNvci4KKworICAgICAgICBGaXhlcyA8aHR0cDovL3dl YmtpdC5vcmcvYi80NTY5Mj4gPHJkYXI6Ly9wcm9ibGVtLzg0MjM0NjQ+IFJFR1JFU1NJT04KKyAg ICAgICAgKHI2MzMzOSk6IE1vdXNlIGN1cnNvciBkaXNhcHBlYXJzIHdoZW4gaG9sZGluZyBtb3Vz ZSBidXR0b24gZG93biBvbgorICAgICAgICBwYWdlCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJDb3JlU3VwcG9ydC9XZWJDaHJvbWVDbGllbnQu Y3BwOgorICAgICAgICAoV2ViQ2hyb21lQ2xpZW50OjpzZXRDdXJzb3IpOiBBZnRlciB0aGUgY3Vy c29yIGlzIHNldCwgcmVnYXJkbGVzcyBvZgorICAgICAgICB3aGV0aGVyIHRoZSBVSSBkZWxlZ2F0 ZSBzZXRzIGl0IG9yIHdlIHNldCBpdCwgcmVjb3JkIHRoZSBjdXJzb3IgdGhhdAorICAgICAgICB3 YXMganVzdCBzZXQuIFRoYXQgd2F5IHdlJ2xsIGJlIGFibGUgdG8gdXNlIHRoZSBjdXJzb3IgbGF0 ZXIgd2hlbgorICAgICAgICByZXNwb25kaW5nIHRvIHRoZSBXTV9TRVRDVVJTT1IgbWVzc2FnZS4K KwogMjAxMC0xMC0yOCAgTU9SSVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBPamFuIFZhZmFpLgpkaWZmIC0tZ2l0IGEvV2ViS2l0L3dpbi9XZWJD b3JlU3VwcG9ydC9XZWJDaHJvbWVDbGllbnQuY3BwIGIvV2ViS2l0L3dpbi9XZWJDb3JlU3VwcG9y dC9XZWJDaHJvbWVDbGllbnQuY3BwCmluZGV4IDg4NGQ0ZGMxZTMzZjBmMTJiMDY2MDc3N2M0ZTRm NzExMWJiNzNmODguLjliYzQ3OTk4ODZjYmViYWQ1Y2MxODA3ZDAzOTAxYjczYjhkNTczOTggMTAw NjQ0Ci0tLSBhL1dlYktpdC93aW4vV2ViQ29yZVN1cHBvcnQvV2ViQ2hyb21lQ2xpZW50LmNwcAor KysgYi9XZWJLaXQvd2luL1dlYkNvcmVTdXBwb3J0L1dlYkNocm9tZUNsaWVudC5jcHAKQEAgLTc5 NywxNyArNzk3LDE5IEBAIHZvaWQgV2ViQ2hyb21lQ2xpZW50OjpzZXRDdXJzb3IoY29uc3QgQ3Vy c29yJiBjdXJzb3IpCiAgICAgaWYgKCFwbGF0Zm9ybUN1cnNvcikKICAgICAgICAgcmV0dXJuOwog CisgICAgYm9vbCBzaG91bGRTZXRDdXJzb3IgPSB0cnVlOwogICAgIGlmIChDT01QdHI8SVdlYlVJ RGVsZWdhdGU+IGRlbGVnYXRlID0gdWlEZWxlZ2F0ZSgpKSB7CiAgICAgICAgIENPTVB0cjxJV2Vi VUlEZWxlZ2F0ZVByaXZhdGU+IGRlbGVnYXRlUHJpdmF0ZShRdWVyeSwgZGVsZWdhdGUpOwogICAg ICAgICBpZiAoZGVsZWdhdGVQcml2YXRlKSB7CiAgICAgICAgICAgICBpZiAoU1VDQ0VFREVEKGRl bGVnYXRlUHJpdmF0ZS0+d2ViVmlld1NldEN1cnNvcihtX3dlYlZpZXcsIHJlaW50ZXJwcmV0X2Nh c3Q8T0xFX0hBTkRMRT4ocGxhdGZvcm1DdXJzb3IpKSkpCi0gICAgICAgICAgICAgICAgcmV0dXJu OworICAgICAgICAgICAgICAgIHNob3VsZFNldEN1cnNvciA9IGZhbHNlOwogICAgICAgICB9CiAg ICAgfQogCi0gICAgbV93ZWJWaWV3LT5zZXRMYXN0Q3Vyc29yKHBsYXRmb3JtQ3Vyc29yKTsKLSAg ICA6OlNldEN1cnNvcihwbGF0Zm9ybUN1cnNvcik7Ci0gICAgcmV0dXJuOworICAgIGlmIChzaG91 bGRTZXRDdXJzb3IpCisgICAgICAgIDo6U2V0Q3Vyc29yKHBsYXRmb3JtQ3Vyc29yKTsKKworICAg IHNldExhc3RTZXRDdXJzb3JUb0N1cnJlbnRDdXJzb3IoKTsKIH0KIAogdm9pZCBXZWJDaHJvbWVD bGllbnQ6OnNldExhc3RTZXRDdXJzb3JUb0N1cnJlbnRDdXJzb3IoKQo=