35259 2010-02-22 13:37:17 -0800 [Qt] QGraphicsScene mousePressEvent does not set the clickCausedFocus flag 2010-03-30 13:11:05 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) All S60 3rd edition RESOLVED FIXED Qt P2 Minor --- 1 joseph.ligman webkit-qt-unassigned commit-queue hausmann kenneth kent.hansen laszlo.gombos tonikitoo oldest_to_newest 192595 0 joseph.ligman 2010-02-22 13:37:17 -0800 Probably not too likely, but this flag not being set has impact on clients using QGraphicsWebView and QStyle::RSIP_OnMouseClickAndAlreadyFocused. And there are no tests for these flags, which would be good to add as well. 200789 1 kent.hansen 2010-03-17 08:08:21 -0700 Hi Joseph, Could you add a testcase that demonstrates the issue? Thanks. 200803 2 joseph.ligman 2010-03-17 08:34:21 -0700 Hi Kent, This is probably a minor issue. If you look at the following lines in qwebpage.cpp, you can see the issue. To me it would make sense to remove the handleSoftwareInputPanel altogether and just let the clients request it when needed. QGraphicsSceneMouseEvent 771 void QWebPagePrivate::mousePressEvent(QGraphicsSceneMouseEvent* ev) 772 { 773 WebCore::Frame* frame = QWebFramePrivate::core(mainFrame); 774 if (!frame->view()) 775 return; 776 777 if (tripleClickTimer.isActive() 778 && (ev->pos().toPoint() - tripleClick).manhattanLength() 779 < QApplication::startDragDistance()) { 780 mouseTripleClickEvent(ev); 781 return; 782 } 783 784 bool accepted = false; 785 PlatformMouseEvent mev(ev, 1); 786 // ignore the event if we can't map Qt's mouse buttons to WebCore::MouseButton 787 if (mev.button() != NoButton) 788 accepted = frame->eventHandler()->handleMousePressEvent(mev); 789 ev->setAccepted(accepted); 790 } QMouseEvent 792 void QWebPagePrivate::mousePressEvent(QMouseEvent *ev) 793 { 822 if (newNode && oldNode != newNode) 823 clickCausedFocus = true; 824 } 928 void QWebPagePrivate::handleSoftwareInputPanel(Qt::MouseButton button) 929 { 930 #if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0) 931 Frame* frame = page->focusController()->focusedFrame(); 932 if (!frame) 933 return; 934 935 if (client && client->inputMethodEnabled() 936 && frame->document()->focusedNode() 937 && button == Qt::LeftButton && qApp->autoSipEnabled()) { 938 QStyle::RequestSoftwareInputPanel behavior = QStyle::RequestSoftwareInputPanel( 939 client->ownerWidget()->style()->styleHint(QStyle::SH_RequestSoftwareInputPanel)); 940 if (!clickCausedFocus || behavior == QStyle::RSIP_OnMouseClick) { 941 QEvent event(QEvent::RequestSoftwareInputPanel); 942 QApplication::sendEvent(client->ownerWidget(), &event); 943 } 944 } 945 946 clickCausedFocus = false; 947 #endif 948 } 200812 3 hausmann 2010-03-17 08:46:10 -0700 You're right, that's indeed missing 206056 4 52051 joseph.ligman 2010-03-30 10:21:19 -0700 Created attachment 52051 Proposed patch 206060 5 tonikitoo 2010-03-30 10:27:40 -0700 (In reply to comment #4) > Created an attachment (id=52051) [details] > Proposed patch I think it looks good, but code could be improved. you do "page->focusController()->focusedFrame()" four times. It would be stored in aux vars, so it would like nicer. 206061 6 tonikitoo 2010-03-30 10:29:47 -0700 (In reply to comment #5) > (In reply to comment #4) > > Created an attachment (id=52051) [details] [details] > > Proposed patch > > I think it looks good, but code could be improved. > > you do "page->focusController()->focusedFrame()" four times. It would be stored > in aux vars, so it would like nicer. actually 6 times :) also, here + RefPtr<WebCore::Node> oldNode; are you sure you want to keep hold a reference just to compare it later one? it can be destroyed when unfocused, no? 206064 7 kenneth 2010-03-30 10:33:45 -0700 > + RefPtr<WebCore::Node> oldNode; > > are you sure you want to keep hold a reference just to compare it later one? > it can be destroyed when unfocused, no? He is not, they get out of scope after the method ends. 206141 8 52051 commit-queue 2010-03-30 13:11:00 -0700 Comment on attachment 52051 Proposed patch Clearing flags on attachment: 52051 Committed r56805: <http://trac.webkit.org/changeset/56805> 206142 9 commit-queue 2010-03-30 13:11:05 -0700 All reviewed patches have been landed. Closing bug. 52051 2010-03-30 10:21:19 -0700 2010-03-30 13:11:00 -0700 Proposed patch bug35259.patch text/plain 2160 joseph.ligman SW5kZXg6IFdlYktpdC9xdC9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0L3F0L0NoYW5n ZUxvZwkocmV2aXNpb24gNTY3OTQpCisrKyBXZWJLaXQvcXQvQ2hhbmdlTG9nCSh3b3JraW5nIGNv cHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTAtMDMtMzAgIEpvZSBMaWdtYW4gIDxqb3NlcGgubGln bWFuQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBbUXRdIFFHcmFwaGljc1NjZW5lIG1vdXNlUHJlc3NFdmVudCBkb2VzIG5vdCBzZXQg dGhlIGNsaWNrQ2F1c2VkRm9jdXMgZmxhZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MzUyNTkKKworICAgICAgICBUaGUgY2xpY2tDYXVzZWRGb2N1cyBm bGFnIGlzIG5vdCBiZWluZyBzZXQgaW4gbWV0aG9kCisgICAgICAgIG1vdXNlUHJlc3NFdmVudChR R3JhcGhpY3NTY2VuZU1vdXNlRXZlbnQqIGV2KS4gVGhpcyBmbGFnIGlzIHVzZWQgCisgICAgICAg IGluIGNvbmp1bmN0aW9uIHdpdGggUVN0eWxlOjpSU0lQX09uTW91c2VDbGlja0FuZEFscmVhZHlG b2N1c2VkIHdoZW4KKyAgICAgICAgZGVjaWRpbmcgdG8gbGF1bmNoIHRoZSBzb2Z0d2FyZSBpbnB1 dCBwYW5lbC4KKworICAgICAgICAqIEFwaS9xd2VicGFnZS5jcHA6CisgICAgICAgIChRV2ViUGFn ZVByaXZhdGU6Om1vdXNlUHJlc3NFdmVudCk6CisKIDIwMTAtMDMtMzAgIEx1aXogQWdvc3Rpbmkg IDxsdWl6LmFnb3N0aW5pQG9wZW5ib3NzYS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2Vu bmV0aCBSb2hkZSBDaHJpc3RpYW5zZW4uCkluZGV4OiBXZWJLaXQvcXQvQXBpL3F3ZWJwYWdlLmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBXZWJLaXQvcXQvQXBpL3F3ZWJwYWdlLmNwcAkocmV2aXNpb24gNTY3 OTQpCisrKyBXZWJLaXQvcXQvQXBpL3F3ZWJwYWdlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNzk3 LDYgKzc5NywxMSBAQCB2b2lkIFFXZWJQYWdlUHJpdmF0ZTo6bW91c2VQcmVzc0V2ZW50KFFHCiAg ICAgaWYgKCFmcmFtZS0+dmlldygpKQogICAgICAgICByZXR1cm47CiAKKyAgICBSZWZQdHI8V2Vi Q29yZTo6Tm9kZT4gb2xkTm9kZTsKKyAgICBpZiAocGFnZS0+Zm9jdXNDb250cm9sbGVyKCktPmZv Y3VzZWRGcmFtZSgpCisgICAgICAgICYmIHBhZ2UtPmZvY3VzQ29udHJvbGxlcigpLT5mb2N1c2Vk RnJhbWUoKS0+ZG9jdW1lbnQoKSkKKyAgICAgICAgb2xkTm9kZSA9IHBhZ2UtPmZvY3VzQ29udHJv bGxlcigpLT5mb2N1c2VkRnJhbWUoKS0+ZG9jdW1lbnQoKS0+Zm9jdXNlZE5vZGUoKTsKKwogICAg IGlmICh0cmlwbGVDbGlja1RpbWVyLmlzQWN0aXZlKCkKICAgICAgICAgICAgICYmIChldi0+cG9z KCkudG9Qb2ludCgpIC0gdHJpcGxlQ2xpY2spLm1hbmhhdHRhbkxlbmd0aCgpCiAgICAgICAgICAg ICAgICAgPCBRQXBwbGljYXRpb246OnN0YXJ0RHJhZ0Rpc3RhbmNlKCkpIHsKQEAgLTgxMCw2ICs4 MTUsMTQgQEAgdm9pZCBRV2ViUGFnZVByaXZhdGU6Om1vdXNlUHJlc3NFdmVudChRRwogICAgIGlm IChtZXYuYnV0dG9uKCkgIT0gTm9CdXR0b24pCiAgICAgICAgIGFjY2VwdGVkID0gZnJhbWUtPmV2 ZW50SGFuZGxlcigpLT5oYW5kbGVNb3VzZVByZXNzRXZlbnQobWV2KTsKICAgICBldi0+c2V0QWNj ZXB0ZWQoYWNjZXB0ZWQpOworCisgICAgUmVmUHRyPFdlYkNvcmU6Ok5vZGU+IG5ld05vZGU7Cisg ICAgaWYgKHBhZ2UtPmZvY3VzQ29udHJvbGxlcigpLT5mb2N1c2VkRnJhbWUoKQorICAgICAgICAm JiBwYWdlLT5mb2N1c0NvbnRyb2xsZXIoKS0+Zm9jdXNlZEZyYW1lKCktPmRvY3VtZW50KCkpCisg ICAgICAgIG5ld05vZGUgPSBwYWdlLT5mb2N1c0NvbnRyb2xsZXIoKS0+Zm9jdXNlZEZyYW1lKCkt PmRvY3VtZW50KCktPmZvY3VzZWROb2RlKCk7CisKKyAgICBpZiAobmV3Tm9kZSAmJiBvbGROb2Rl ICE9IG5ld05vZGUpCisgICAgICAgIGNsaWNrQ2F1c2VkRm9jdXMgPSB0cnVlOwogfQogCiB2b2lk IFFXZWJQYWdlUHJpdmF0ZTo6bW91c2VQcmVzc0V2ZW50KFFNb3VzZUV2ZW50ICpldikK