Bug 30549 – [Qt] Infinite loop (leading to crash) when setting cursor in QGraphicsWebView

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Antonio Gomes]

The http://trac.webkit.org/changeset/49782 commit has reveal a bug in QGWV's setCursor method.

* Problem (manual backtrace):

_1) Widget[Qt]::setCursor
_2) QWebPageClient::setCursor
_3) QGraphicsWebViewPrivate::updateCursor
_4) QGraphicsItem::setCursor.
_5) QGraphicsWidget::itemChange method is called as the first action of qgraphicsitem.cpp::setCursor (see qt/src/gui/graphicsview/qgraphicsitem.cpp).
_6) QGraphicsWidget::itemChange fires QApplication::sendEvent(CursorChange), that is captured by QGWV's ::event.

At this point QGWV cursor has not been set yet (remember we are in the middle of _3_ yet).

_7) the quote below is executed:

void QGraphicsWebView::event() {
    (...)
    if (event->type() == QEvent::CursorChange) {
        if (cursor().shape() == Qt::ArrowCursor)
        d->resetCursor();
    }
    (...)
}

_8) d->resetCursor goes to 3) again, in an infinite loop ....... crash !

Solution:
When QGraphicsItem::setCursor calls QGraphicsWidget::itemChange as its first action, it passes 'CursorChange' as 'change'. However we can not emit 'CursoChange' event yet (see loop above).

At the end of setCursor method (when cursor had already been set), QGraphicsWidget::itemChange method is called again, but now passing the 'CursorHasChanged' as 'change'. This is the time we have to act in QGraphicsWebView.

patch coming ...

[Antonio Gomes]

Created attachment 41470 [details]
(commit in r49846) patch 0.1 - late emission of CursorChange event.

[Antonio Gomes]

QWebView works because QWidget emits the CursorChange event just like the proposed patch does (at the end of the setCursor method):

void QWidget::setCursor(const QCursor &cursor)
{
    Q_D(QWidget);
    (...)
    setAttribute(Qt::WA_SetCursor);
    d->setCursor_sys(cursor);

    QEvent event(QEvent::CursorChange);
    QApplication::sendEvent(this, &event);
}

[Kenneth Rohde Christiansen]

Seems like a workaround. Shouldn't this be fixed in Qt instead? 

Right now it seems that with QWidgets, the event is being called after the cursor has been set, and that that is not the case with QGraphicsWidgets. That is different behaviour and if that is not intentional (which I guess not), it can be considered a bug.

[Antonio Gomes]

Comment on attachment 41470 [details]
(commit in r49846) patch 0.1 - late emission of CursorChange event.

clearing r+ flag since patch has landed.

[Antonio Gomes]

as i talked to ariya on irc and pointed out in bug description and comment #2, i agree w/ kenneth that it has to be investigated in qt side about why such behaviour, however i do not need we have to be crashy until then. So i am landing the fix/workaround and filing to followup bugs:

1) one for auto test this
2) for investigating the real reason on why qt differ from qwidget to qgraphicsitem here.

[Antonio Gomes]

(In reply to comment #5)
> as i talked to ariya on irc and pointed out in bug description and comment #2,
> i agree w/ kenneth that it has to be investigated in qt side about why such
> behaviour, however i do not need we have to be crashy until then. So i am
> landing the fix/workaround and filing to followup bugs:
> 
> 1) one for auto test this
> 2) for investigating the real reason on why qt differ from qwidget to
> qgraphicsitem here.

1) 30557

2) 30558