31351 2009-11-11 05:45:38 -0800 [Qt] Remove the resetCursor code in qwebview and qgraphicswebview 2009-11-11 06:48:34 -0800 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC OS X 10.5 RESOLVED INVALID Qt P2 Normal --- 30557 1 tonikitoo webkit-unassigned oldest_to_newest 162457 0 tonikitoo 2009-11-11 05:45:38 -0800 Both qwebview and qgraphicswebview ::event() methods have the same code: (...) #ifndef QT_NO_CURSOR #if QT_VERSION >= 0x040400 if (event->type() == QEvent::CursorChange) { // An unsetCursor will set the cursor to Qt::ArrowCursor. // Thus this cursor change might be a QWidget::unsetCursor() // If this is not the case and it came from WebCore, the // QWebPageClient already has set its cursor internally // to Qt::ArrowCursor, so updating the cursor is always // right, as it falls back to the last cursor set by // WebCore. // FIXME: Add a QEvent::CursorUnset or similar to Qt. if (cursor().shape() == Qt::ArrowCursor) d->resetCursor(); } #endif #endif (...) it is pointless, since cursor implementation has changed w/ qwebpageclient, and make calls to "unsetCursor" to fail. I would propose to remove these code, and unsetCursor starts to work. Not sure if it'd go to 4.6 though. 162458 1 42957 tonikitoo 2009-11-11 05:51:02 -0800 Created attachment 42957 patch 0.1 patch removes this unsetCursor call from both ::event methods in qwebview and qgraphicswebview. bug 30557 has auto tests for it. 162463 2 tonikitoo 2009-11-11 06:18:05 -0800 follows a more detailed explanation about why the code is wrong. suppose: 0) one is in a qwebview context. 1) mouse moves hover a link and goes to Qt::PointingHandCursor shape. 2) unsetCursor is called. e.g "view.unsetCursor();". 3) cursor changes internally in qwidget::unsetCursor (i.e. it goes to Qt::ArrowCursor shape) , a "CursorChange" event is fired and qwebview::event method fetch the event. 4) since cursor is already in Qt::ArrowCursor shape, it goes in the "IF" statement below: (...) if (event->type() == QEvent::CursorChange) { if (cursor().shape() == Qt::ArrowCursor) d->resetCursor(); } (...) QWebPageClient::resetCursor in its turn will call setCursor(m_lastCursor) and set cursor back to Qt::PointingHandCursor shape. In the end, the initial unsetCursor call is useless (see _2_) since cursor ends up in the same shape, not as Qt::ArrowCursor. 162464 3 tonikitoo 2009-11-11 06:21:02 -0800 furthermore, for qgraphicswebview the problem described in comment #2 would not happen just because qgraphicsitem's unsetCursor does not fire a CursorChange event as qwidget's do, and then qgraphicswebview ::event wont be triggered. so the problem is hidden, imho 162467 4 tonikitoo 2009-11-11 06:48:34 -0800 after discussing w/ kenneth on IRC about the desired behavior, then it turns that is is not what this bug proposes, then it is INVALID. I will file another bug to track the real problem. 42957 2009-11-11 05:51:02 -0800 2009-11-11 05:51:02 -0800 patch 0.1 0001--Qt-Remove-the-resetCursor-code-in-qwebview-and-qgr.patch text/plain 4206 tonikitoo RnJvbSAyNzg0N2E3OTI4NDNhNTJkNTA1YWQwMmM1N2JkNDAxN2Q1NTU5MGViIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbnRvbmlvIEdvbWVzIDx0b25pa2l0b29Ad2Via2l0Lm9yZz4K RGF0ZTogV2VkLCAxMSBOb3YgMjAwOSAwOTo0OTowMSAtMDQwMApTdWJqZWN0OiBbUEFUQ0hdIFtR dF0gUmVtb3ZlIHRoZSByZXNldEN1cnNvciBjb2RlIGluIHF3ZWJ2aWV3IGFuZCBxZ3JhcGhpY3N3 ZWJ2aWV3CiBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzEzNTEKClJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKQ2FsbGluZyB1bnNldCBjdXJzb3IgYXMgaXMgaW4g OjpldmVudCBtZXRob2QgbWFrZSB1bnNldEN1cnNvciB0byBmYWlsLgpUaGUgY3VycmVudCBpbXBs ZW1lbnRhdGlvbiBpcyBub3cgcG9pbnRsZXNzIHNpbmNlIFFXZWJQYWdlQ2xpZW50IGhhbmRsZXMK Y3Vyc29yIHNldCBpbnRlcm5hbGx5LgoKUGF0Y2ggcmVtb3ZlcyB0aGlzIGNvZGUuCgoqIEFwaS9x Z3JhcGhpY3N3ZWJ2aWV3LmNwcDoKKFFHcmFwaGljc1dlYlZpZXc6OmV2ZW50KToKKiBBcGkvcXdl YnZpZXcuY3BwOgooUVdlYlZpZXc6OmV2ZW50KToKLS0tCiBXZWJLaXQvcXQvQXBpL3FncmFwaGlj c3dlYnZpZXcuY3BwIHwgICAxNiAtLS0tLS0tLS0tLS0tLS0tCiBXZWJLaXQvcXQvQXBpL3F3ZWJ2 aWV3LmNwcCAgICAgICAgIHwgICAxOSArKy0tLS0tLS0tLS0tLS0tLS0tCiBXZWJLaXQvcXQvQ2hh bmdlTG9nICAgICAgICAgICAgICAgIHwgICAxOCArKysrKysrKysrKysrKysrKysKIDMgZmlsZXMg Y2hhbmdlZCwgMjAgaW5zZXJ0aW9ucygrKSwgMzMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEv V2ViS2l0L3F0L0FwaS9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcCBiL1dlYktpdC9xdC9BcGkvcWdyYXBo aWNzd2Vidmlldy5jcHAKaW5kZXggZDFmMzIxNC4uOTIxOTIxNCAxMDA2NDQKLS0tIGEvV2ViS2l0 L3F0L0FwaS9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcAorKysgYi9XZWJLaXQvcXQvQXBpL3FncmFwaGlj c3dlYnZpZXcuY3BwCkBAIC0yOTQsMjIgKzI5NCw2IEBAIGJvb2wgUUdyYXBoaWNzV2ViVmlldzo6 ZXZlbnQoUUV2ZW50KiBldmVudCkKICAgICAgICAgfSBlbHNlCiAjZW5kaWYgLy8gUVRfTk9fQ09O VEVYVE1FTlUKICAgICAgICAgewotI2lmbmRlZiBRVF9OT19DVVJTT1IKLSNpZiBRVF9WRVJTSU9O ID49IDB4MDQwNDAwCi0gICAgICAgICAgICBpZiAoZXZlbnQtPnR5cGUoKSA9PSBRRXZlbnQ6OkN1 cnNvckNoYW5nZSkgewotICAgICAgICAgICAgICAgIC8vIEFuIHVuc2V0Q3Vyc29yIHdpbGwgc2V0 IHRoZSBjdXJzb3IgdG8gUXQ6OkFycm93Q3Vyc29yLgotICAgICAgICAgICAgICAgIC8vIFRodXMg dGhpcyBjdXJzb3IgY2hhbmdlIG1pZ2h0IGJlIGEgUVdpZGdldDo6dW5zZXRDdXJzb3IoKQotICAg ICAgICAgICAgICAgIC8vIElmIHRoaXMgaXMgbm90IHRoZSBjYXNlIGFuZCBpdCBjYW1lIGZyb20g V2ViQ29yZSwgdGhlCi0gICAgICAgICAgICAgICAgLy8gUVdlYlBhZ2VDbGllbnQgYWxyZWFkeSBo YXMgc2V0IGl0cyBjdXJzb3IgaW50ZXJuYWxseQotICAgICAgICAgICAgICAgIC8vIHRvIFF0OjpB cnJvd0N1cnNvciwgc28gdXBkYXRpbmcgdGhlIGN1cnNvciBpcyBhbHdheXMKLSAgICAgICAgICAg ICAgICAvLyByaWdodCwgYXMgaXQgZmFsbHMgYmFjayB0byB0aGUgbGFzdCBjdXJzb3Igc2V0IGJ5 Ci0gICAgICAgICAgICAgICAgLy8gV2ViQ29yZS4KLSAgICAgICAgICAgICAgICAvLyBGSVhNRTog QWRkIGEgUUV2ZW50OjpDdXJzb3JVbnNldCBvciBzaW1pbGFyIHRvIFF0LgotICAgICAgICAgICAg ICAgIGlmIChjdXJzb3IoKS5zaGFwZSgpID09IFF0OjpBcnJvd0N1cnNvcikKLSAgICAgICAgICAg ICAgICAgICAgZC0+cmVzZXRDdXJzb3IoKTsKLSAgICAgICAgICAgIH0KLSNlbmRpZgotI2VuZGlm CiAgICAgICAgIH0KICAgICB9CiAgICAgcmV0dXJuIFFHcmFwaGljc1dpZGdldDo6ZXZlbnQoZXZl bnQpOwpkaWZmIC0tZ2l0IGEvV2ViS2l0L3F0L0FwaS9xd2Vidmlldy5jcHAgYi9XZWJLaXQvcXQv QXBpL3F3ZWJ2aWV3LmNwcAppbmRleCAxMmIyMGFiLi42ODA2NzFmIDEwMDY0NAotLS0gYS9XZWJL aXQvcXQvQXBpL3F3ZWJ2aWV3LmNwcAorKysgYi9XZWJLaXQvcXQvQXBpL3F3ZWJ2aWV3LmNwcApA QCAtNjMzLDI0ICs2MzMsOSBAQCBib29sIFFXZWJWaWV3OjpldmVudChRRXZlbnQgKmUpCiAgICAg ICAgICAgICBkLT5wYWdlLT51cGRhdGVQb3NpdGlvbkRlcGVuZGVudEFjdGlvbnMoZXZlbnQtPnBv cygpKTsKICAgICAgICAgfSBlbHNlCiAjZW5kaWYgLy8gUVRfTk9fQ09OVEVYVE1FTlUKLSAgICAg ICAgaWYgKGUtPnR5cGUoKSA9PSBRRXZlbnQ6OlNob3J0Y3V0T3ZlcnJpZGUpIHsKKyAgICAgICAg aWYgKGUtPnR5cGUoKSA9PSBRRXZlbnQ6OlNob3J0Y3V0T3ZlcnJpZGUpCiAgICAgICAgICAgICBk LT5wYWdlLT5ldmVudChlKTsKLSNpZm5kZWYgUVRfTk9fQ1VSU09SCi0jaWYgUVRfVkVSU0lPTiA+ PSAweDA0MDQwMAotICAgICAgICB9IGVsc2UgaWYgKGUtPnR5cGUoKSA9PSBRRXZlbnQ6OkN1cnNv ckNoYW5nZSkgewotICAgICAgICAgICAgLy8gQW4gdW5zZXRDdXJzb3Igd2lsbCBzZXQgdGhlIGN1 cnNvciB0byBRdDo6QXJyb3dDdXJzb3IuCi0gICAgICAgICAgICAvLyBUaHVzIHRoaXMgY3Vyc29y IGNoYW5nZSBtaWdodCBiZSBhIFFXaWRnZXQ6OnVuc2V0Q3Vyc29yKCkKLSAgICAgICAgICAgIC8v IElmIHRoaXMgaXMgbm90IHRoZSBjYXNlIGFuZCBpdCBjYW1lIGZyb20gV2ViQ29yZSwgdGhlCi0g ICAgICAgICAgICAvLyBRV2ViUGFnZUNsaWVudCBhbHJlYWR5IGhhcyBzZXQgaXRzIGN1cnNvciBp bnRlcm5hbGx5Ci0gICAgICAgICAgICAvLyB0byBRdDo6QXJyb3dDdXJzb3IsIHNvIHVwZGF0aW5n IHRoZSBjdXJzb3IgaXMgYWx3YXlzCi0gICAgICAgICAgICAvLyByaWdodCwgYXMgaXQgZmFsbHMg YmFjayB0byB0aGUgbGFzdCBjdXJzb3Igc2V0IGJ5Ci0gICAgICAgICAgICAvLyBXZWJDb3JlLgot ICAgICAgICAgICAgLy8gRklYTUU6IEFkZCBhIFFFdmVudDo6Q3Vyc29yVW5zZXQgb3Igc2ltaWxh ciB0byBRdC4KLSAgICAgICAgICAgIGlmIChjdXJzb3IoKS5zaGFwZSgpID09IFF0OjpBcnJvd0N1 cnNvcikKLSAgICAgICAgICAgICAgICBkLT5wYWdlLT5kLT5jbGllbnQtPnJlc2V0Q3Vyc29yKCk7 Ci0jZW5kaWYKLSNlbmRpZgotICAgICAgICB9IGVsc2UgaWYgKGUtPnR5cGUoKSA9PSBRRXZlbnQ6 OkxlYXZlKQorICAgICAgICBlbHNlIGlmIChlLT50eXBlKCkgPT0gUUV2ZW50OjpMZWF2ZSkKICAg ICAgICAgICAgIGQtPnBhZ2UtPmV2ZW50KGUpOwogICAgIH0KIApkaWZmIC0tZ2l0IGEvV2ViS2l0 L3F0L0NoYW5nZUxvZyBiL1dlYktpdC9xdC9DaGFuZ2VMb2cKaW5kZXggYzE4ZjI4ZS4uMTVkYzgz ZSAxMDA2NDQKLS0tIGEvV2ViS2l0L3F0L0NoYW5nZUxvZworKysgYi9XZWJLaXQvcXQvQ2hhbmdl TG9nCkBAIC0xLDMgKzEsMjEgQEAKKzIwMDktMTEtMTEgIEFudG9uaW8gR29tZXMgIDx0b25pa2l0 b29Ad2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBbUXRdIFJlbW92ZSB0aGUgcmVzZXRDdXJzb3IgY29kZSBpbiBxd2VidmlldyBhbmQg cWdyYXBoaWNzd2VidmlldworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MzEzNTEKKworICAgICAgICBDYWxsaW5nIHVuc2V0IGN1cnNvciBhcyBpcyBpbiA6 OmV2ZW50IG1ldGhvZCBtYWtlIHVuc2V0Q3Vyc29yIHRvIGZhaWwuCisgICAgICAgIFRoZSBjdXJy ZW50IGltcGxlbWVudGF0aW9uIGlzIG5vdyBwb2ludGxlc3Mgc2luY2UgUVdlYlBhZ2VDbGllbnQg aGFuZGxlcworICAgICAgICBjdXJzb3Igc2V0IGludGVybmFsbHkuCisKKyAgICAgICAgUGF0Y2gg cmVtb3ZlcyB0aGlzIGNvZGUuCisKKyAgICAgICAgKiBBcGkvcWdyYXBoaWNzd2Vidmlldy5jcHA6 CisgICAgICAgIChRR3JhcGhpY3NXZWJWaWV3OjpldmVudCk6CisgICAgICAgICogQXBpL3F3ZWJ2 aWV3LmNwcDoKKyAgICAgICAgKFFXZWJWaWV3OjpldmVudCk6CisKIDIwMDktMTEtMDkgIExhc3ps byBHb21ib3MgIDxsYXN6bG8uMS5nb21ib3NAbm9raWEuY29tPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IEtlbm5ldGggUm9oZGUgQ2hyaXN0aWFuc2VuLgotLSAKMS42LjAuNAoK