QDockWidget: Remove "group" bool trap

The unplug() and startDrag() functions of QMainWindowLayout and
QDockWidget used a boolean argument specifying whether a single dock
widget or a group of dock widgets should be unplugged. The argument
defaulted to true.

That has lead to inconsistent unplug operations, broken item_lists and
crashes, especially when the methods were called without an argument.

To improve code readability, replace bool trap with a meaningful
enum. Remove default arguments, in order to force explicit calls.

This patch does not change behavior, it is just carved out to
facilitate reviews.

Task-number: QTBUG-118578
Task-number: QTBUG-118579
Pick-to: 6.6 6.5
Change-Id: I50341b055f0bb76c2797b2fb1126a10de1fee7dd
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Axel Spoerl 2023-11-08 16:50:35 +01:00
parent c02f8b9d4d
commit c93ab8c2a0
5 changed files with 19 additions and 13 deletions

View File

@ -750,7 +750,7 @@ void QDockWidgetPrivate::initDrag(const QPoint &pos, bool nca)
tabbed widgets, and false if the dock widget should always be dragged
alone.
*/
void QDockWidgetPrivate::startDrag(bool group)
void QDockWidgetPrivate::startDrag(DragScope scope)
{
Q_Q(QDockWidget);
@ -764,7 +764,7 @@ void QDockWidgetPrivate::startDrag(bool group)
bool wasFloating = q->isFloating();
#endif
state->widgetItem = layout->unplug(q, group);
state->widgetItem = layout->unplug(q, scope);
if (state->widgetItem == nullptr) {
/* Dock widget has a QMainWindow parent, but was never inserted with
QMainWindow::addDockWidget, so the QMainWindowLayout has no
@ -978,7 +978,7 @@ bool QDockWidgetPrivate::mouseMoveEvent(QMouseEvent *event)
} else
#endif
{
startDrag();
startDrag(DragScope::Group);
q->grabMouse();
ret = true;
}
@ -1089,7 +1089,7 @@ void QDockWidgetPrivate::nonClientAreaMouseEvent(QMouseEvent *event)
break;
state->ctrlDrag = (event->modifiers() & Qt::ControlModifier) ||
(!hasFeature(this, QDockWidget::DockWidgetMovable) && q->isFloating());
startDrag();
startDrag(DragScope::Group);
break;
case QEvent::NonClientAreaMouseMove:
if (state == nullptr || !state->dragging)

View File

@ -51,6 +51,11 @@ class QDockWidgetPrivate : public QWidgetPrivate
};
public:
enum class DragScope {
Group,
Widget
};
void init();
void toggleView(bool);
void toggleTopLevel();
@ -86,7 +91,7 @@ public:
void setWindowState(bool floating, bool unplug = false, const QRect &rect = QRect());
void nonClientAreaMouseEvent(QMouseEvent *event);
void initDrag(const QPoint &pos, bool nca);
void startDrag(bool group = true);
void startDrag(DragScope scope);
void endDrag(bool abort = false);
void moveEvent(QMoveEvent *event);
void recalculatePressPos(QResizeEvent *event);

View File

@ -1792,7 +1792,7 @@ void QMainWindowTabBar::mouseMoveEvent(QMouseEvent *e)
QDockWidgetPrivate *dockPriv = static_cast<QDockWidgetPrivate *>(QObjectPrivate::get(draggingDock));
QDockWidgetLayout *dwlayout = static_cast<QDockWidgetLayout *>(draggingDock->layout());
dockPriv->initDrag(dwlayout->titleArea().center(), true);
dockPriv->startDrag(false);
dockPriv->startDrag(QDockWidgetPrivate::DragScope::Widget);
if (dockPriv->state)
dockPriv->state->ctrlDrag = e->modifiers() & Qt::ControlModifier;
}
@ -2552,25 +2552,25 @@ static QTabBar::Shape tabwidgetPositionToTabBarShape(QWidget *w)
Returns the QLayoutItem of the dragged element.
The layout item is kept in the layout but set as a gap item.
*/
QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, bool group)
QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, QDockWidgetPrivate::DragScope scope)
{
#if QT_CONFIG(dockwidget) && QT_CONFIG(tabwidget)
auto *groupWindow = qobject_cast<const QDockWidgetGroupWindow *>(widget->parentWidget());
if (!widget->isWindow() && groupWindow) {
if (group && groupWindow->tabLayoutInfo()) {
if (scope == QDockWidgetPrivate::DragScope::Group && groupWindow->tabLayoutInfo()) {
// We are just dragging a floating window as it, not need to do anything, we just have to
// look up the corresponding QWidgetItem* if it exists
if (QDockAreaLayoutInfo *info = dockInfo(widget->parentWidget())) {
QList<int> groupWindowPath = info->indexOf(widget->parentWidget());
return groupWindowPath.isEmpty() ? nullptr : info->item(groupWindowPath).widgetItem;
}
qCDebug(lcQpaDockWidgets) << "Drag only:" << widget << "Group:" << group;
qCDebug(lcQpaDockWidgets) << "Drag only:" << widget << "Group:" << (scope == QDockWidgetPrivate::DragScope::Group);
return nullptr;
}
QList<int> path = groupWindow->layoutInfo()->indexOf(widget);
QDockAreaLayoutItem parentItem = groupWindow->layoutInfo()->item(path);
QLayoutItem *item = parentItem.widgetItem;
if (group && path.size() > 1
if (scope == QDockWidgetPrivate::DragScope::Group && path.size() > 1
&& unplugGroup(this, &item, parentItem)) {
qCDebug(lcQpaDockWidgets) << "Unplugging:" << widget << "from" << item;
return item;
@ -2663,7 +2663,7 @@ QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, bool group)
if (QDockWidget *dw = qobject_cast<QDockWidget*>(widget)) {
Q_ASSERT(path.constFirst() == 1);
#if QT_CONFIG(tabwidget)
if (group && (dockOptions & QMainWindow::GroupedDragging) && path.size() > 3
if (scope == QDockWidgetPrivate::DragScope::Group && (dockOptions & QMainWindow::GroupedDragging) && path.size() > 3
&& unplugGroup(this, &item,
layoutState.dockAreaLayout.item(path.mid(1, path.size() - 2)))) {
path.removeLast();

View File

@ -29,6 +29,7 @@
#include "QtCore/qset.h"
#include "private/qlayoutengine_p.h"
#include "private/qwidgetanimator_p.h"
#include "private/qdockwidget_p.h"
#if QT_CONFIG(dockwidget)
#include "qdockarealayout_p.h"
@ -571,7 +572,7 @@ public:
void hover(QLayoutItem *hoverTarget, const QPoint &mousePos);
bool plug(QLayoutItem *widgetItem);
QLayoutItem *unplug(QWidget *widget, bool group = false);
QLayoutItem *unplug(QWidget *widget, QDockWidgetPrivate::DragScope scope);
void revert(QLayoutItem *widgetItem);
void applyState(QMainWindowLayoutState &newState, bool animate = true);
void restore(bool keepSavedState = false);

View File

@ -185,7 +185,7 @@ void QToolBarPrivate::startDrag(bool moving)
#endif
if (!moving) {
state->widgetItem = layout->unplug(q);
state->widgetItem = layout->unplug(q, QDockWidgetPrivate::DragScope::Group);
Q_ASSERT(state->widgetItem != nullptr);
}
state->dragging = !moving;