23989 2009-02-17 05:08:22 -0800 The cursor behavior in GTK is wrong 2009-02-18 04:17:58 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED Gtk P2 Normal --- 0 techrazy.yang webkit-unassigned xan.lopez oldest_to_newest 109958 0 techrazy.yang 2009-02-17 05:08:22 -0800 The Wibkit Gtk use WidgetPrivate::cursor to store the current cursor, but the cursor is get through a C++ static variable, and this cause something wrong, when there is frame (the key point is that there are many widget and then many WidgetPrivate::cursor, but the cursor static variable only one). When the cursor cross frame side, it may not be changed... I will upload a testcase to display the problem and a patch which fix it. 109960 1 27726 techrazy.yang 2009-02-17 05:25:49 -0800 Created attachment 27726 The testcase file. Please open the test file. It will create a two columns frame, with each on open the http://www.webkit.org/. Then , move your mouse across the two frames splitter, you will find the cursor will display incorrectly. I mean, when the mouse is on the splitter, it should be a double arrow, else it should be a pointer. But, WebKit GTK display the cursor wrong. 109961 2 27727 techrazy.yang 2009-02-17 05:27:46 -0800 Created attachment 27727 The first version Change the WidgetPriate::cursor to a static memeber, that means the whole process should only use one cursor at one time, this is rational. 109990 3 xan.lopez 2009-02-17 11:47:13 -0800 I think this just fixes the bug by accident. By using the same cached cursor for all widgets, and initializing it to 0 on each instance, you effectively disable it once per widget created. Also, how does making the cache per-class makes sense? Could you elaborate? 110042 4 techrazy.yang 2009-02-17 18:34:25 -0800 (In reply to comment #3) > I think this just fixes the bug by accident. By using the same cached cursor > for all widgets, and initializing it to 0 on each instance, you effectively > disable it once per widget created. Also, how does making the cache per-class > makes sense? Could you elaborate? Cache per-class means cache per-application. One application hold one cursor type at one time. Make the cursor a static variable, we can trace the cursor type trough all the widgets of the application. I think this make sense. Disable it once per widget created indeed make no sense. I forget to delete that line in the patch. I have upload a new patch, thanks. 110043 5 27741 techrazy.yang 2009-02-17 18:35:05 -0800 Created attachment 27741 The second version 110051 6 xan.lopez 2009-02-18 00:40:24 -0800 (In reply to comment #4) > (In reply to comment #3) > > I think this just fixes the bug by accident. By using the same cached cursor > > for all widgets, and initializing it to 0 on each instance, you effectively > > disable it once per widget created. Also, how does making the cache per-class > > makes sense? Could you elaborate? > > Cache per-class means cache per-application. One application hold one cursor > type at one time. Make the cursor a static variable, we can trace the cursor > type trough all the widgets of the application. I think this make sense. > Disable it once per widget created indeed make no sense. I forget to delete > that line in the patch. I have upload a new patch, thanks. > Well, sure, I was hoping for the rationale of how this fixes the problem. I think I get it now though: the issue is with widgets that get set a cursor A, then the user moves elsewhere with the cursor changing, and when he tries to come back and set cursor A again the cache says "no, I have it already", and nothing happens. So making the cache global indeed fixes the issue (and saves memory...). The last patch looks good to me. 110057 7 27743 xan.lopez 2009-02-18 01:21:29 -0800 Created attachment 27743 cursor.patch Make the cursor cache just a static file variable and remove some stale code, pointed out by Mark Rowe. 110058 8 27743 mrowe 2009-02-18 01:25:30 -0800 Comment on attachment 27743 cursor.patch > +static GdkCursor* lastSetCursor = 0; There's no need to explicitly initialise this to zero. > @@ -85,11 +74,11 @@ void Widget::setCursor(const Cursor& cursor) > // gdk_window_set_cursor() in certain GDK backends seems to be an > // expensive operation, so avoid it if possible. > > - if (pcur == m_data->cursor) > + if (pcur == lastSetCursor) > return; > > gdk_window_set_cursor(gdkDrawable(platformWidget()) ? GDK_WINDOW(gdkDrawable(platformWidget())) : GTK_WIDGET(root()->hostWindow()->platformWindow())->window, pcur); > - m_data->cursor = pcur; > + lastSetCursor = pcur; > } Ugh, does this code really use pcur as a variable name? :-( r=me 110061 9 techrazy.yang 2009-02-18 02:10:18 -0800 (In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #3) > > > I think this just fixes the bug by accident. By using the same cached cursor > > > for all widgets, and initializing it to 0 on each instance, you effectively > > > disable it once per widget created. Also, how does making the cache per-class > > > makes sense? Could you elaborate? > > > > Cache per-class means cache per-application. One application hold one cursor > > type at one time. Make the cursor a static variable, we can trace the cursor > > type trough all the widgets of the application. I think this make sense. > > Disable it once per widget created indeed make no sense. I forget to delete > > that line in the patch. I have upload a new patch, thanks. > > > > Well, sure, I was hoping for the rationale of how this fixes the problem. I > think I get it now though: the issue is with widgets that get set a cursor A, > then the user moves elsewhere with the cursor changing, and when he tries to > come back and set cursor A again the cache says "no, I have it already", and > nothing happens. So making the cache global indeed fixes the issue (and saves > memory...). Right! 110065 10 gustavo 2009-02-18 04:17:58 -0800 Landed in r41055. 27726 2009-02-17 05:25:49 -0800 2009-02-17 05:25:49 -0800 The testcase file. frameset.html text/html 172 techrazy.yang PGh0bWw+DQo8ZnJhbWVzZXQgY29scz0iNTAlLDUwJSI+DQogIDxmcmFtZSBpZD0ibGVmdEZyYW1l IiBzcmM9Imh0dHA6Ly93d3cud2Via2l0Lm9yZyI+DQogIDxmcmFtZSBpZD0icmlnaHRGcmFtZSIg c3JjPSJodHRwOi8vd3d3LndlYmtpdC5vcmciPg0KPC9mcmFtZXNldD4NCjwvaHRtbD4NCg0KDQoN Cg== 27727 2009-02-17 05:27:46 -0800 2009-02-17 18:35:05 -0800 The first version 23989.patch text/plain 931 techrazy.yang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0MTAzOCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTEgQEAKKzIwMDktMDItMTcgIEJvIFlhbmcgIDx0ZWNocmF6eS55YW5nQGdtYWls LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM5ODkKKworICAgICAgICAq IHBsYXRmb3JtL2d0ay9XaWRnZXRHdGsuY3BwOgorCiAyMDA5LTAyLTE3ICBHdXN0YXZvIE5vcm9u aGEgU2lsdmEgIDxnbnNAZ25vbWUub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFk bGVyLgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9ndGsvV2lkZ2V0R3RrLmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2d0ay9XaWRnZXRHdGsuY3BwCShyZXZpc2lvbiA0MTAzNikK KysrIFdlYkNvcmUvcGxhdGZvcm0vZ3RrL1dpZGdldEd0ay5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTQzLDkgKzQzLDExIEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogY2xhc3MgV2lkZ2V0UHJpdmF0 ZSB7CiBwdWJsaWM6Ci0gICAgR2RrQ3Vyc29yKiBjdXJzb3I7CisgICAgc3RhdGljIEdka0N1cnNv ciogY3Vyc29yOwogfTsKIAorR2RrQ3Vyc29yKiBXaWRnZXRQcml2YXRlOjpjdXJzb3IgPSAwOwor CiBXaWRnZXQ6OldpZGdldChQbGF0Zm9ybVdpZGdldCB3aWRnZXQpCiAgICAgOiBtX2RhdGEobmV3 IFdpZGdldFByaXZhdGUpCiB7Cg== 27741 2009-02-17 18:35:05 -0800 2009-02-18 01:21:29 -0800 The second version 23989.patch text/plain 1003 techrazy.yang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0MTA1MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTEgQEAKKzIwMDktMDItMTcgIEJvIFlhbmcgIDx0ZWNocmF6eS55YW5nQGdtYWls LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM5ODkKKworICAgICAgICAq IHBsYXRmb3JtL2d0ay9XaWRnZXRHdGsuY3BwOgorCiAyMDA5LTAyLTE3ICBEYXZpZCBMZXZpbiAg PGxldmluQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbGV4ZXkgUHJvc2t1 cnlha292LgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9ndGsvV2lkZ2V0R3RrLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2d0ay9XaWRnZXRHdGsuY3BwCShyZXZpc2lvbiA0MTA1 MCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vZ3RrL1dpZGdldEd0ay5jcHAJKHdvcmtpbmcgY29weSkK QEAgLTQzLDE0ICs0MywxNSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKIGNsYXNzIFdpZGdldFBy aXZhdGUgewogcHVibGljOgotICAgIEdka0N1cnNvciogY3Vyc29yOworICAgIHN0YXRpYyBHZGtD dXJzb3IqIGN1cnNvcjsKIH07CiAKK0dka0N1cnNvciogV2lkZ2V0UHJpdmF0ZTo6Y3Vyc29yID0g MDsKKwogV2lkZ2V0OjpXaWRnZXQoUGxhdGZvcm1XaWRnZXQgd2lkZ2V0KQogICAgIDogbV9kYXRh KG5ldyBXaWRnZXRQcml2YXRlKQogewogICAgIGluaXQod2lkZ2V0KTsKLSAgICBtX2RhdGEtPmN1 cnNvciA9IDA7CiB9CiAKIFdpZGdldDo6fldpZGdldCgpCg== 27743 2009-02-18 01:21:29 -0800 2009-02-18 01:25:30 -0800 cursor.patch cursor.patch text/plain 3784 xan.lopez RnJvbSBhMWI5ZTEzMzFmYjRkNTFhMjg5ZTRjOTg3MzVmZjQyNjRkYWFkNjhmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYYW4gTG9wZXogPHhhbkBnbm9tZS5vcmc+CkRhdGU6IFdlZCwg MTggRmViIDIwMDkgMTE6MTk6MTIgKzAyMDAKU3ViamVjdDogW1BBVENIXSAyMDA5LTAyLTE4ICBY YW4gTG9wZXogIDx4YW5AZ25vbWUub3JnPgoKICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KCiAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIz OTg5CgogICAgICAgIEJhc2VkIG9uIGEgcGF0Y2ggYnkgQm8gWWFuZyAgPHRlY2hyYXp5LnlhbmdA Z21haWwuY29tPgoKICAgICAgICBNYWtlIHRoZSBjdXJzb3IgY2FjaGUgZ2xvYmFsLCB0aGF0J3Mg YWxsIHdlIHJlYWxseSBuZWVkIGFuZAogICAgICAgIG90aGVyd2lzZSB3ZSBjYW4gbWlzcyBjdXJz b3IgdHJhbnNpdGlvbnMgaW4gc29tZSBzaXR1YXRpb25zIChzZWUKICAgICAgICB0aGUgYnVnIGZv ciBvbmUgdGVzdGNhc2UpLiBBbHNvIHJlbW92ZSBzb21lIG5vdyB1c2VsZXNzIGNvZGUuCgogICAg ICAgICogcGxhdGZvcm0vV2lkZ2V0Lmg6CiAgICAgICAgKiBwbGF0Zm9ybS9ndGsvV2lkZ2V0R3Rr LmNwcDoKICAgICAgICAoV2ViQ29yZTo6V2lkZ2V0OjpXaWRnZXQpOgogICAgICAgIChXZWJDb3Jl OjpXaWRnZXQ6On5XaWRnZXQpOgogICAgICAgIChXZWJDb3JlOjpXaWRnZXQ6OnNldEN1cnNvcik6 Ci0tLQogV2ViQ29yZS9DaGFuZ2VMb2cgICAgICAgICAgICAgICAgICB8ICAgMTggKysrKysrKysr KysrKysrKysrCiBXZWJDb3JlL3BsYXRmb3JtL1dpZGdldC5oICAgICAgICAgIHwgICAgMyArLS0K IFdlYkNvcmUvcGxhdGZvcm0vZ3RrL1dpZGdldEd0ay5jcHAgfCAgIDE3ICsrKy0tLS0tLS0tLS0t LS0tCiAzIGZpbGVzIGNoYW5nZWQsIDIyIGluc2VydGlvbnMoKyksIDE2IGRlbGV0aW9ucygtKQoK ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ODQ5ZmI2Mi4uNDg2M2FiNiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMSBAQAorMjAwOS0wMi0xOCAgWGFuIExvcGV6ICA8 eGFuQGdub21lLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM5ODkKKwor ICAgICAgICBCYXNlZCBvbiBhIHBhdGNoIGJ5IEJvIFlhbmcgIDx0ZWNocmF6eS55YW5nQGdtYWls LmNvbT4KKworICAgICAgICBNYWtlIHRoZSBjdXJzb3IgY2FjaGUgZ2xvYmFsLCB0aGF0J3MgYWxs IHdlIHJlYWxseSBuZWVkIGFuZAorICAgICAgICBvdGhlcndpc2Ugd2UgY2FuIG1pc3MgY3Vyc29y IHRyYW5zaXRpb25zIGluIHNvbWUgc2l0dWF0aW9ucyAoc2VlCisgICAgICAgIHRoZSBidWcgZm9y IG9uZSB0ZXN0Y2FzZSkuIEFsc28gcmVtb3ZlIHNvbWUgbm93IHVzZWxlc3MgY29kZS4KKworICAg ICAgICAqIHBsYXRmb3JtL1dpZGdldC5oOgorICAgICAgICAqIHBsYXRmb3JtL2d0ay9XaWRnZXRH dGsuY3BwOgorICAgICAgICAoV2ViQ29yZTo6V2lkZ2V0OjpXaWRnZXQpOgorICAgICAgICAoV2Vi Q29yZTo6V2lkZ2V0Ojp+V2lkZ2V0KToKKyAgICAgICAgKFdlYkNvcmU6OldpZGdldDo6c2V0Q3Vy c29yKToKKwogMjAwOS0wMi0xNyAgQWRhbSBSb2JlbiAgPGFyb2JlbkBhcHBsZS5jb20+CiAKICAg ICAgICAgV2luZG93cyBidWlsZCBmaXgKZGlmZiAtLWdpdCBhL1dlYkNvcmUvcGxhdGZvcm0vV2lk Z2V0LmggYi9XZWJDb3JlL3BsYXRmb3JtL1dpZGdldC5oCmluZGV4IDJmMjIwNjkuLjAyOWNjNzgg MTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxhdGZvcm0vV2lkZ2V0LmgKKysrIGIvV2ViQ29yZS9wbGF0 Zm9ybS9XaWRnZXQuaApAQCAtMTM3LDcgKzEzNyw2IEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIHZv aWQgc2V0Rm9jdXMoKTsKIAogICAgIHZvaWQgc2V0Q3Vyc29yKGNvbnN0IEN1cnNvciYpOwotICAg IEN1cnNvciBjdXJzb3IoKTsKIAogICAgIHZpcnR1YWwgdm9pZCBzaG93KCk7CiAgICAgdmlydHVh bCB2b2lkIGhpZGUoKTsKQEAgLTE5Myw3ICsxOTIsNyBAQCBwcml2YXRlOgogICAgIAogICAgIElu dFJlY3QgbV9mcmFtZTsgLy8gTm90IHVzZWQgd2hlbiBhIG5hdGl2ZSB3aWRnZXQgZXhpc3RzLgog Ci0jaWYgUExBVEZPUk0oTUFDKSB8fCBQTEFURk9STShHVEspCisjaWYgUExBVEZPUk0oTUFDKQog ICAgIFdpZGdldFByaXZhdGUqIG1fZGF0YTsKICNlbmRpZgogfTsKZGlmZiAtLWdpdCBhL1dlYkNv cmUvcGxhdGZvcm0vZ3RrL1dpZGdldEd0ay5jcHAgYi9XZWJDb3JlL3BsYXRmb3JtL2d0ay9XaWRn ZXRHdGsuY3BwCmluZGV4IDgyZmVkNzQuLmI2NTgyMTQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxh dGZvcm0vZ3RrL1dpZGdldEd0ay5jcHAKKysrIGIvV2ViQ29yZS9wbGF0Zm9ybS9ndGsvV2lkZ2V0 R3RrLmNwcApAQCAtNDEsMjMgKzQxLDE3IEBACiAKIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAotY2xh c3MgV2lkZ2V0UHJpdmF0ZSB7Ci1wdWJsaWM6Ci0gICAgR2RrQ3Vyc29yKiBjdXJzb3I7Ci19Owor c3RhdGljIEdka0N1cnNvciogbGFzdFNldEN1cnNvciA9IDA7CiAKIFdpZGdldDo6V2lkZ2V0KFBs YXRmb3JtV2lkZ2V0IHdpZGdldCkKLSAgICA6IG1fZGF0YShuZXcgV2lkZ2V0UHJpdmF0ZSkKIHsK ICAgICBpbml0KHdpZGdldCk7Ci0gICAgbV9kYXRhLT5jdXJzb3IgPSAwOwogfQogCiBXaWRnZXQ6 On5XaWRnZXQoKQogewogICAgIEFTU0VSVCghcGFyZW50KCkpOwogICAgIHJlbGVhc2VQbGF0Zm9y bVdpZGdldCgpOwotICAgIGRlbGV0ZSBtX2RhdGE7CiB9CiAKIHZvaWQgV2lkZ2V0OjpzZXRGb2N1 cygpCkBAIC02NSwxMSArNTksNiBAQCB2b2lkIFdpZGdldDo6c2V0Rm9jdXMoKQogICAgIGd0a193 aWRnZXRfZ3JhYl9mb2N1cyhwbGF0Zm9ybVdpZGdldCgpID8gcGxhdGZvcm1XaWRnZXQoKSA6IEdU S19XSURHRVQocm9vdCgpLT5ob3N0V2luZG93KCktPnBsYXRmb3JtV2luZG93KCkpKTsKIH0KIAot Q3Vyc29yIFdpZGdldDo6Y3Vyc29yKCkKLXsKLSAgICByZXR1cm4gQ3Vyc29yKG1fZGF0YS0+Y3Vy c29yKTsKLX0KLQogc3RhdGljIEdka0RyYXdhYmxlKiBnZGtEcmF3YWJsZShQbGF0Zm9ybVdpZGdl dCB3aWRnZXQpCiB7CiAgICAgcmV0dXJuIHdpZGdldCA/IHdpZGdldC0+d2luZG93IDogMDsKQEAg LTg1LDExICs3NCwxMSBAQCB2b2lkIFdpZGdldDo6c2V0Q3Vyc29yKGNvbnN0IEN1cnNvciYgY3Vy c29yKQogICAgIC8vIGdka193aW5kb3dfc2V0X2N1cnNvcigpIGluIGNlcnRhaW4gR0RLIGJhY2tl bmRzIHNlZW1zIHRvIGJlIGFuCiAgICAgLy8gZXhwZW5zaXZlIG9wZXJhdGlvbiwgc28gYXZvaWQg aXQgaWYgcG9zc2libGUuCiAKLSAgICBpZiAocGN1ciA9PSBtX2RhdGEtPmN1cnNvcikKKyAgICBp ZiAocGN1ciA9PSBsYXN0U2V0Q3Vyc29yKQogICAgICAgICByZXR1cm47CiAKICAgICBnZGtfd2lu ZG93X3NldF9jdXJzb3IoZ2RrRHJhd2FibGUocGxhdGZvcm1XaWRnZXQoKSkgPyBHREtfV0lORE9X KGdka0RyYXdhYmxlKHBsYXRmb3JtV2lkZ2V0KCkpKSA6IEdUS19XSURHRVQocm9vdCgpLT5ob3N0 V2luZG93KCktPnBsYXRmb3JtV2luZG93KCkpLT53aW5kb3csIHBjdXIpOwotICAgIG1fZGF0YS0+ Y3Vyc29yID0gcGN1cjsKKyAgICBsYXN0U2V0Q3Vyc29yID0gcGN1cjsKIH0KIAogdm9pZCBXaWRn ZXQ6OnNob3coKQotLSAKMS42LjEKCg==