Bug 55568 – [Qt] QGraphicsWebView should use updateMicroFocus() of QGraphicsItem

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Alexis Menard (darktears)]

Rather than it's own implementation for obvious reason like bugfixing.

Otherwise we can get some issues like :

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff352be98 in QScopedPointer<QObjectData, QScopedPointerDeleter<QObjectData> >::data (this=0x8) at ../../include/QtCore/../../../qt-47-src/src/corelib/tools/qscopedpointer.h:135
135             return d;
(gdb) bt
#0  0x00007ffff352be98 in QScopedPointer<QObjectData, QScopedPointerDeleter<QObjectData> >::data (this=0x8) at ../../include/QtCore/../../../qt-47-src/src/corelib/tools/qscopedpointer.h:135
#1  0x00007ffff3ce556c in qGetPtrHelper<QScopedPointer<QObjectData> > (p=...) at ../../include/QtCore/../../../qt-47-src/src/corelib/global/qglobal.h:2338
#2  0x00007ffff3ce56f0 in QGraphicsScene::d_func (this=0x0) at /home/darktears/dev/troll/qt-47-src/src/gui/graphicsview/qgraphicsscene.h:297
#3  0x00007ffff3cd5e80 in QGraphicsScene::views (this=0x0) at /home/darktears/dev/troll/qt-47-src/src/gui/graphicsview/qgraphicsscene.cpp:3308
#4  0x00007ffff6a1bf3e in QGraphicsWebViewPrivate::_q_updateMicroFocus() () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#5  0x00007ffff6a1c23a in QGraphicsWebView::qt_metacall(QMetaObject::Call, int, void**) () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#6  0x00007fffe3bba200 in GraphicsWebView::qt_metacall(QMetaObject::Call, int, void**) () from /home/darktears/dev/troll/webkit-trunk/Release/imports/QtWebKit/libqmlwebkitplugin.so
#7  0x00007ffff2bcec77 in QMetaObject::metacall (object=0x71d520, cl=QMetaObject::InvokeMetaMethod, idx=33, argv=0x7fffffff8ce0) at /home/darktears/dev/troll/qt-47-src/src/corelib/kernel/qmetaobject.cpp:237
#8  0x00007ffff2be3d53 in QMetaObject::activate (sender=0x7271f0, m=0x7ffff7c9e5c0, local_signal_index=18, argv=0x0) at /home/darktears/dev/troll/qt-47-src/src/corelib/kernel/qobject.cpp:3278
#9  0x00007ffff6a4c381 in WebCore::EditorClientQt::setInputMethodState(bool) () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#10 0x00007ffff6e3ad57 in WebCore::FocusController::setFocusedNode(WebCore::Node*, WTF::PassRefPtr<WebCore::Frame>) () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#11 0x00007ffff6bd126b in WebCore::Element::focus(bool) () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#12 0x00007ffff66034e2 in WebCore::jsElementPrototypeFunctionFocus(JSC::ExecState*) () from /home/darktears/dev/troll/webkit-trunk/Release/lib/libQtWebKit.so.4
#13 0x00007fff978c21b8 in ?? ()
#14 0xffff000000000000 in ?? ()
#15 0x00007fff978ce33f in ?? ()
#16 0x00000000000004e8 in ?? ()
#17 0x00007fffe0109810 in ?? ()
#18 0x00007fff97461520 in ?? ()
#19 0x0000000000000008 in ?? ()
#20 0x00007fff974615d0 in ?? ()
#21 0x00007fffe01095d0 in ?? ()
#22 0x0000000000000000 in ?? ()
(gdb) quit
A debugging session is active.

Catched by the base implementation but not the one in QGraphicsWebView. Couldn't reproduce this crash with reliability but it's obvious to understand that a JS event arrive but the view is already out of the scene and the code was not taking into account that use case.

[Alexis Menard (darktears)]

Created attachment 84398 [details]
Patch to fix the issue

[Benjamin Poulain]

Comment on attachment 84398 [details]
Patch to fix the issue

View in context: https://bugs.webkit.org/attachment.cgi?id=84398&action=review

We unfortunately still support 4.6. You should #ifdef for now, so that code can be removed as soon as we release 4.8.

> Source/WebKit/qt/Api/qgraphicswebview.cpp:475
>      connect(d->page, SIGNAL(microFocusChanged()),
> -            this, SLOT(_q_updateMicroFocus()));
> +            this, SLOT(updateMicroFocus()));

This could go on a single line. We don't have 80 char limit for WebKit.

[Alexis Menard (darktears)]

(In reply to comment #2)
> (From update of attachment 84398 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84398&action=review
> 
> We unfortunately still support 4.6. You should #ifdef for now, so that code can be removed as soon as we release 4.8.

We don't. QtWebKit 2.2 which is the incoming trunk is not supporting it.

2.1 and 2.1.x yes but they won't cherry pick this patch.

For people with 4.6 configuration they have to go for QtWebKit 2.0.

Benedikte sent an email about that.

> 
> > Source/WebKit/qt/Api/qgraphicswebview.cpp:475
> >      connect(d->page, SIGNAL(microFocusChanged()),
> > -            this, SLOT(_q_updateMicroFocus()));
> > +            this, SLOT(updateMicroFocus()));
> 
> This could go on a single line. We don't have 80 char limit for WebKit.

Will modify.

[Benjamin Poulain]

Comment on attachment 84398 [details]
Patch to fix the issue

Back to r? since 4.6 is now out of the picture.

[Andreas Kling]

Comment on attachment 84398 [details]
Patch to fix the issue

LGTM.

[WebKit Commit Bot]

Comment on attachment 84398 [details]
Patch to fix the issue

Clearing flags on attachment: 84398

Committed r80270: <http://trac.webkit.org/changeset/80270>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.