RESOLVED FIXED 35259
[Qt] QGraphicsScene mousePressEvent does not set the clickCausedFocus flag
https://bugs.webkit.org/show_bug.cgi?id=35259
Summary [Qt] QGraphicsScene mousePressEvent does not set the clickCausedFocus flag
Joseph Ligman
Reported 2010-02-22 13:37:17 PST
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.
Attachments
Proposed patch (2.11 KB, patch)
2010-03-30 10:21 PDT, Joseph Ligman
no flags
Kent Hansen
Comment 1 2010-03-17 08:08:21 PDT
Hi Joseph, Could you add a testcase that demonstrates the issue? Thanks.
Joseph Ligman
Comment 2 2010-03-17 08:34:21 PDT
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 }
Simon Hausmann
Comment 3 2010-03-17 08:46:10 PDT
You're right, that's indeed missing
Joseph Ligman
Comment 4 2010-03-30 10:21:19 PDT
Created attachment 52051 [details] Proposed patch
Antonio Gomes
Comment 5 2010-03-30 10:27:40 PDT
(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.
Antonio Gomes
Comment 6 2010-03-30 10:29:47 PDT
(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?
Kenneth Rohde Christiansen
Comment 7 2010-03-30 10:33:45 PDT
> + 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.
WebKit Commit Bot
Comment 8 2010-03-30 13:11:00 PDT
Comment on attachment 52051 [details] Proposed patch Clearing flags on attachment: 52051 Committed r56805: <http://trac.webkit.org/changeset/56805>
WebKit Commit Bot
Comment 9 2010-03-30 13:11:05 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.