1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ensure QWidget::destroy() events and attributes match hiding the widget

Browse Source 
When destroying a widget via QWidget::destroy(), we clear WA_WState_Created,
and then delete the QWidgetWindow, which in turn hides the widget. But
QWidgetPrivate::setVisible(false) skips hide_helper() if the widget has
not been created, which leaves out important events such as the hide
event, and even more important, fails to clear WA_WState_Visible. As
a result, the widget is left visible (and mapped), even if it has been
destroyed.

This is normally not a big issue for the main use of destroy(), namely
destructing the widget, but for cases where destroy() and create() is
used to recreate the widget this is important.

We now unconditionally call hide_helper() if the widget is not already
hidden. As a result, the widget will correctly be visible=false after a
destroy(). This in turn means we need to re-apply the visible state after
recreating the widget when we detect a mismatch in RHI configuration.

Due to our meddling of the Hidden and ExplicitShowHide attributes in
QWidgetWindow private, a QWidet::show() will not have any effect after
being destroy(). This is okey for now, as destroy() is internal to
a widget, and we make sure to either update WA_WState_Visible and
WA_WState_Hidden (in QWidgetPrivate::setParent_sys), or use the
QWidgetPrivate::setVisible() code path directly, which doesn't
have that issue. The root problem will be fixed in a follow up.

Change-Id: I77cb88d75e57f0d9a31741161fb14d618a653291
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 371d7ea19a8075a1ad2c2011433b40e8eb1f6816)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Tor Arne Vestbø 2024-02-06 16:36:17 +01:00 committed by Qt Cherry-pick Bot
parent fe434acac6
commit 0531e444a7
2 changed files with 110 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/widgets/kernel/qwidget.cpp
View File

@ -8430,8 +8430,7 @@ void QWidgetPrivate::setVisible(bool visible)
if (!q->testAttribute(Qt::WA_WState_Hidden)) {
q->setAttribute(Qt::WA_WState_Hidden);
if (q->testAttribute(Qt::WA_WState_Created))
hide_helper();
hide_helper();
}
// invalidate layout similar to updateGeometry()
@ -10899,10 +10898,12 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f)
}
if (recreate) {
const auto windowStateBeforeDestroy = newtlw->windowState();
const auto visibilityBeforeDestroy = newtlw->isVisible();
newtlw->destroy();
newtlw->create();
Q_ASSERT(newtlw->windowHandle());
newtlw->windowHandle()->setWindowStates(windowStateBeforeDestroy);
QWidgetPrivate::get(newtlw)->setVisible(visibilityBeforeDestroy);
}
}
}

135
tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
View File

@ -122,6 +122,34 @@ static QByteArray msgComparisonFailed(T v1, const char *op, T v2)
return s.toLocal8Bit();
}
template<class T> class EventSpy : public QObject
{
public:
EventSpy(T *widget, QEvent::Type event)
: m_widget(widget), eventToSpy(event)
{
if (m_widget)
m_widget->installEventFilter(this);
}
T *widget() const { return m_widget; }
int count() const { return m_count; }
void clear() { m_count = 0; }
protected:
bool eventFilter(QObject *object, QEvent *event) override
{
if (event->type() == eventToSpy)
++m_count;
return QObject::eventFilter(object, event);
}
private:
T *m_widget;
const QEvent::Type eventToSpy;
int m_count = 0;
};
Q_LOGGING_CATEGORY(lcTests, "qt.widgets.tests")
class tst_QWidget : public QObject
@ -227,6 +255,7 @@ private slots:
void ensureCreated();
void createAndDestroy();
void eventsAndAttributesOnDestroy();
void winIdChangeEvent();
void persistentWinId();
void showNativeChild();
@ -5262,6 +5291,84 @@ void tst_QWidget::createAndDestroy()
QVERIFY(widget.internalWinId());
}
void tst_QWidget::eventsAndAttributesOnDestroy()
{
// The events and attributes when destroying a widget should
// include those of hiding the widget.
CreateDestroyWidget widget;
EventSpy<QWidget> showEventSpy(&widget, QEvent::Show);
EventSpy<QWidget> hideEventSpy(&widget, QEvent::Hide);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), false);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), false);
QCOMPARE(widget.testAttribute(Qt::WA_Mapped), false);
widget.show();
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), true);
QTRY_COMPARE(widget.testAttribute(Qt::WA_Mapped), true);
QCOMPARE(showEventSpy.count(), 1);
QCOMPARE(hideEventSpy.count(), 0);
widget.hide();
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), false);
QCOMPARE(widget.testAttribute(Qt::WA_Mapped), false);
QCOMPARE(showEventSpy.count(), 1);
QCOMPARE(hideEventSpy.count(), 1);
widget.show();
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), true);
QTRY_COMPARE(widget.testAttribute(Qt::WA_Mapped), true);
QCOMPARE(showEventSpy.count(), 2);
QCOMPARE(hideEventSpy.count(), 1);
widget.destroy();
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), false);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), false);
QCOMPARE(widget.testAttribute(Qt::WA_Mapped), false);
QCOMPARE(showEventSpy.count(), 2);
QCOMPARE(hideEventSpy.count(), 2);
const int hideEventsAfterDestroy = hideEventSpy.count();
widget.create();
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), false);
QCOMPARE(widget.testAttribute(Qt::WA_Mapped), false);
QCOMPARE(showEventSpy.count(), 2);
QCOMPARE(hideEventSpy.count(), hideEventsAfterDestroy);
QWidgetPrivate::get(&widget)->setVisible(true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), true);
QTRY_COMPARE(widget.testAttribute(Qt::WA_Mapped), true);
QCOMPARE(showEventSpy.count(), 3);
QCOMPARE(hideEventSpy.count(), hideEventsAfterDestroy);
// Make sure the destroy that happens when a top level
// is moved to being a child does not prevent the child
// being shown again.
QWidget parent;
QWidget child;
parent.show();
QVERIFY(QTest::qWaitForWindowExposed(&parent));
child.show();
QVERIFY(QTest::qWaitForWindowExposed(&child));
child.setParent(&parent);
QCOMPARE(child.testAttribute(Qt::WA_WState_Created), false);
QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), false);
child.show();
QCOMPARE(child.testAttribute(Qt::WA_WState_Created), true);
QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), true);
QVERIFY(QTest::qWaitForWindowExposed(&child));
}
void tst_QWidget::winIdChangeEvent()
{
{
@ -7060,34 +7167,6 @@ void tst_QWidget::setFocus()
}
}
template<class T> class EventSpy : public QObject
{
public:
EventSpy(T *widget, QEvent::Type event)
: m_widget(widget), eventToSpy(event)
{
if (m_widget)
m_widget->installEventFilter(this);
}
T *widget() const { return m_widget; }
int count() const { return m_count; }
void clear() { m_count = 0; }
protected:
bool eventFilter(QObject *object, QEvent *event) override
{
if (event->type() == eventToSpy)
++m_count;
return QObject::eventFilter(object, event);
}
private:
T *m_widget;
const QEvent::Type eventToSpy;
int m_count = 0;
};
#ifndef QT_NO_CURSOR
void tst_QWidget::setCursor()
{