Review Request: KTabBar: Don't crash on middle-clicking tabs with Qt 4.5.2

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1017/
-----------------------------------------------------------

Review request for kdelibs.


Summary
-------

The combination of a QTabBar change in Qt 4.5.2 and the recent restoration of middle-click tab moving in KTabBars which have the movable attribute set leads to a crash if a tab is middle-clicked before the first "tab move" operation, see

http://bugs.kde.org/show_bug.cgi?id=198075

I think the reason is as follows:

1. If the middle mouse button is pressed on a tab, KTabBar::mousePressEvent() creates a "faked" QMouseEvent (with middle button replaced by left button) and sends that to itself to make QTabBar think that the left mouse button was pressed and a tab move operation can be started. 

2. The faked event ends up in QTabBar::mousePressEvent(). In that method, d->pressedIndex is set from the default value -1 to the index of the pressed tab.

[QTabBar::mouseMoveEvent(), which is never called in the problematic use case, uses d->pressedIndex to determine that a tab move operation is in progress, sets d->dragInProgress to true and calls QTabBarPrivate::setupMovableTab() which initialises the QWidget* d->movingTab, which is 0 initially]

3. If the middle mouse button is released *before* the mouse is moved, KTabBar::mouseReleaseEvent() emits a mouseMiddleClick signal and returns. No faked QMouseEvent is generated in this case because d->mMiddleMouseTabMoveInProgress is false unless the mouse is moved between pressing and releasing the button.

[If QTabBar::mouseReleaseEvent() was called now, it would set d->pressedIndex back to its default value -1, indicating that no tab move is in progress].

4. If either QTabBar::mouseMoveEvent() or QTabBar::removeTab() (in case middle clicking is configured to close tabs) are called now, they think a tab move operation is in progress because d->pressedIndex is not -1 and try to finish that operation. We therefore end up in QTabBarPrivate::_q_moveTabFinished(int index), which relies on the QWidget* movingTab being initialised -> we get a segfault.

I would suggest to fix it by also sending a faked QMouseEvent in 3. At the moment, I can't see any other way of resetting d->pressedIndex without breaking other things, but maybe I'm overlooking something.


This addresses bug 198075.
    https://bugs.kde.org/show_bug.cgi?id=198075


Diffs
-----

  /trunk/KDE/kdelibs/kdeui/widgets/ktabbar.cpp 996095 

Diff: http://reviewboard.kde.org/r/1017/diff


Testing
-------

Fixes the crash for me (no matter if middle-clicking is configured to close tabs or not). Everything else (including ktabwidgettest and ktabwidget_unittest) seems to work fine.


Thanks,

Frank