Skip to content

Commit

Permalink
Don't send QEvent::Hide to an already hidden top level widget
Browse files Browse the repository at this point in the history
When hiding a popup by clicking outside of its area, a window is closed.
Depending on the platform specific implementation details, this can
result in multiple calls to QWidgetPrivate::setVisible(false). The first
one from the handling of the close event in QWidgetWindow::event; the
second from the destruction of the window in QWindow::event.

Since the first call already sets the Qt::WA_WState_Hidden flag before
calling QWidgetPrivate::hide_helper, we can test if the flag is set
and skip the second call if it is.

The included test does not reproduce the issue, as that issue only
reproduces if the close event is generated by the mouse event handling
in the Cocoa platform plugin (which doesn't call QWidget::close, but
rather sends a native close event to the platform window). However, it
verifies that the fix doesn't introduce any regressions.

Change-Id: Id0eda9326a8adf0cc1f6a3840f9ac0b635ab39a1
Fixes: QTBUG-79134
Reviewed-by: Qt CI Bot <[email protected]>
Reviewed-by: Tor Arne Vestbø <[email protected]>
(cherry picked from commit ed86a4f)
Reviewed-by: Volker Hilsheimer <[email protected]>
  • Loading branch information
vohi committed May 20, 2020
1 parent 1bd81e8 commit 0786ac8
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/widgets/kernel/qwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8136,9 +8136,11 @@ void QWidgetPrivate::setVisible(bool visible)
if (!q->isWindow() && q->parentWidget()) // && !d->getOpaqueRegion().isEmpty())
q->parentWidget()->d_func()->setDirtyOpaqueRegion();

q->setAttribute(Qt::WA_WState_Hidden);
if (q->testAttribute(Qt::WA_WState_Created))
hide_helper();
if (!q->testAttribute(Qt::WA_WState_Hidden)) {
q->setAttribute(Qt::WA_WState_Hidden);
if (q->testAttribute(Qt::WA_WState_Created))
hide_helper();
}

// invalidate layout similar to updateGeometry()
if (!q->isWindow() && q->parentWidget()) {
Expand Down
74 changes: 74 additions & 0 deletions tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ private slots:
void tst_resize_count();
void tst_move_count();

void tst_showhide_count();

void tst_eventfilter_on_toplevel();

void QTBUG_50561_QCocoaBackingStore_paintDevice_crash();
Expand Down Expand Up @@ -1042,6 +1044,78 @@ void tst_QWidget_window::tst_resize_count()

}

/*!
This test verifies that windows get a balanced number of show
and hide events, no matter how the window was closed.
*/
void tst_QWidget_window::tst_showhide_count()
{
class EventSpy : public QObject
{
public:
EventSpy()
{
QApplication::instance()->installEventFilter(this);
}

int takeCount(QWidget *widget, QEvent::Type type) {
const auto entry = Entry(widget, type);
int count = counter[entry];
counter[entry] = 0;
return count;
}
protected:
bool eventFilter(QObject *receiver, QEvent *event)
{
if (QWidget *widget = qobject_cast<QWidget*>(receiver)) {
const auto entry = Entry(widget, event->type());
++counter[entry];
return false;
}
return QObject::eventFilter(receiver, event);
}
private:
using Entry = QPair<QWidget*, QEvent::Type>;
QHash<Entry, int> counter;
};

EventSpy spy;

QWidget w1;
w1.setGeometry(100, 100, 200, 200);

w1.show();
QCOMPARE(spy.takeCount(&w1, QEvent::Show), 1);
w1.hide();
QCOMPARE(spy.takeCount(&w1, QEvent::Hide), 1);
w1.close();
QCOMPARE(spy.takeCount(&w1, QEvent::Close), 1);
w1.show();
QCOMPARE(spy.takeCount(&w1, QEvent::Show), 1);
w1.close();
QCOMPARE(spy.takeCount(&w1, QEvent::Hide), 1);
QCOMPARE(spy.takeCount(&w1, QEvent::Close), 1);

w1.show();
QWidget *popup = new QWidget(&w1, Qt::Popup);
popup->setGeometry(120, 120, 30, 30);
popup->show();
popup->close();
QCOMPARE(spy.takeCount(popup, QEvent::Show), 1);
QCOMPARE(spy.takeCount(popup, QEvent::Hide), 1);
QCOMPARE(spy.takeCount(popup, QEvent::Close), 1);

popup->show();

// clicking outside the popup should close the popup
QTest::mousePress(popup->window(), Qt::LeftButton, {}, QPoint(-10, -10));

QCOMPARE(spy.takeCount(popup, QEvent::Show), 1);
QCOMPARE(spy.takeCount(popup, QEvent::Hide), 1);
QCOMPARE(spy.takeCount(popup, QEvent::Close), 1);
}


class MoveWidget : public QWidget
{
Q_OBJECT
Expand Down

0 comments on commit 0786ac8

Please sign in to comment.