Bug 35259 – [Qt] QGraphicsScene mousePressEvent does not set the clickCausedFocus flag

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Joseph Ligman]

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.

[Kent Hansen]

Hi Joseph,
Could you add a testcase that demonstrates the issue? Thanks.

[Joseph Ligman]

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]

You're right, that's indeed missing

[Joseph Ligman]

Created attachment 52051 [details]
Proposed patch

[Antonio Gomes]

(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]

(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]

> +    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 on attachment 52051 [details]
Proposed patch

Clearing flags on attachment: 52051

Committed r56805: <http://trac.webkit.org/changeset/56805>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.