mudlet/test/functional_tests/DialogTeardownTest.cpp

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

380 lines
17 KiB
C++
Raw Permalink Normal View History

infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
/***************************************************************************
* Copyright (C) 2026 by Vadim Peretokin - vadim.peretokin@mudlet.org *
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
* This program is distributed in the hope that it will be useful, *
* but WITHOUT ANY WARRANTY; without even the implied warranty of *
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
* GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program; if not, write to the *
* Free Software Foundation, Inc., *
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
***************************************************************************/
/*
* Functional tests for windows that are destroyed while one of their own
* editing widgets still has the keyboard focus (#9574).
*
* A visible window is taken off the screen while its base-class destructors
* unwind (~QDialog hides it, ~QWidget closes any other window class), which
* moves the focus off the widget that holds it; a QLineEdit (QAbstractSpinBox
* and QKeySequenceEdit behave the same) answers that by emitting
* editingFinished() into a slot of a window whose derived part has already
* been destroyed.
*
* A debug build ends the whole run there, on Qt's "Called object is not of the
* correct type (class destructor may have already run)". A release build has
* that assert compiled out and runs the slot against the destroyed object
* instead, so each test also checks that the edit in the focused field was not
* acted on - the same assertion holds whichever way the build was configured.
*
* Run with: ctest -R DialogTeardownTest -V
*/
#include <QtTest/QtTest>
#include <chrono>
#include <QAction>
infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
#include <QKeySequenceEdit>
#include <QLineEdit>
#include <QScopeGuard>
infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
#include "Host.h"
#include "MudletInstanceCoordinator.h"
#include "TelnetServerStub.h"
#include "TriggerUnit.h"
#include "dlgConnectionProfiles.h"
#include "dlgProfilePreferences.h"
#include "dlgTriggerEditor.h"
#include "mudlet.h"
#if defined(INCLUDE_UPDATER)
#include "updater.h"
#endif
infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
using namespace std::chrono_literals;
extern void qInitResources_mudlet();
extern void qInitResources_qm();
extern void qInitResources_additional_splash_screens();
extern void qInitResources_mudlet_fonts_common();
extern void qInitResources_mudlet_fonts_posix();
void initializeQRCResourcesForDialogTeardownTest();
class DialogTeardownTest : public QObject
{
Q_OBJECT
private:
TelnetServerStub* mpServer = nullptr;
Host* mpHost = nullptr;
const QString mProfileName = qsl("DialogTeardown-Test");
QString mPort; // assigned the stub's actual ephemeral port in initTestCase()
const QString mLocalhost = qsl("localhost");
void deleteProfileDirectory(const QString& profileName)
{
const QString path = mudlet::getMudletPath(enums::profileHomePath, profileName);
QDir dir(path);
if (dir.exists()) {
dir.removeRecursively();
}
}
void startProfile(const QString& profileName, const QString& address, const QString& port)
{
QTimer::singleShot(0ms, qApp, [profileName, address, port]() {
mudlet::self()->startAutoLogin({});
QTest::qWait(100ms);
QTest::mouseClick(mudlet::self()->mpConnectionDialog->new_profile_button, Qt::LeftButton);
QTest::qWait(100ms);
QTest::keyClicks(QApplication::focusWidget(), profileName);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
QTest::qWait(100ms);
QTest::keyClicks(QApplication::focusWidget(), address);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
QTest::qWait(100ms);
QTest::keyClicks(QApplication::focusWidget(), port);
QTest::qWait(100ms);
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Return);
});
QSignalSpy spy(mudlet::self(), &mudlet::signal_profileLoaded);
if (!spy.wait(5000)) {
QFAIL("Profile took too long to load.");
}
mpHost = mudlet::self()->getActiveHost();
if (!mpHost) {
QFAIL("No active host available for the test.");
}
}
// Gives the widget the keyboard focus and something to report: a QLineEdit
// only emits editingFinished() on focus-out once its text has been touched,
// and the setText() here is what arms that - a field nothing has written to
// stays quiet and would make this test prove nothing.
void focusWithText(QLineEdit* lineEdit, const QString& text)
{
QVERIFY2(lineEdit->isVisible(), "Field has to be on screen to be able to take the focus");
lineEdit->setText(text);
lineEdit->setFocus();
QCoreApplication::processEvents();
QCOMPARE(QApplication::focusWidget(), lineEdit);
}
private slots:
void initTestCase()
{
initializeQRCResourcesForDialogTeardownTest();
mpServer = new TelnetServerStub(qApp);
mpServer->start(mLocalhost, 0); // ephemeral OS-assigned port avoids collisions across concurrent test runs
mPort = QString::number(mpServer->serverPort());
mudlet::start();
mudlet::self()->setupConfig();
mudlet::self()->takeOwnershipOfInstanceCoordinator(std::make_unique<MudletInstanceCoordinator>(qsl("MudletInstanceCoordinator")));
mudlet::self()->init();
mudlet::self()->setStorePasswordsSecurely(false);
deleteProfileDirectory(mProfileName);
startProfile(mProfileName, mLocalhost, mPort);
QVERIFY2(mpHost, "No active host after profile creation");
}
void cleanupTestCase()
{
mpHost = nullptr;
delete mpServer;
mpServer = nullptr;
deleteProfileDirectory(mProfileName);
delete mudlet::self();
}
// Everything below rests on Qt still emitting the focus-out signals while a
// window is being destroyed. If a future Qt stops doing that the other tests
// would keep passing while testing nothing, so pin the mechanism itself on
// widgets of our own - the receiver here outlives them, which is exactly what
// the windows under test cannot manage.
void test_teardownEmitsTheSignalsThisIsAllAbout()
{
auto* dialog = new QDialog(mudlet::self());
auto* layout = new QVBoxLayout(dialog);
auto* lineEdit = new QLineEdit(dialog);
layout->addWidget(lineEdit);
dialog->show();
lineEdit->setText(qsl("some text"));
lineEdit->setFocus();
QCoreApplication::processEvents();
QCOMPARE(QApplication::focusWidget(), lineEdit);
QSignalSpy lineEditSpy(lineEdit, &QLineEdit::editingFinished);
delete dialog;
QVERIFY2(lineEditSpy.count() == 1,
"A focused QLineEdit no longer reports editingFinished() when its "
"window is destroyed - the rest of this file now proves nothing");
// the same for the shortcut editors the preferences are full of
auto* keySequenceDialog = new QDialog(mudlet::self());
auto* keySequenceLayout = new QVBoxLayout(keySequenceDialog);
auto* secondKeySequenceEdit = new QKeySequenceEdit(keySequenceDialog);
keySequenceLayout->addWidget(secondKeySequenceEdit);
keySequenceDialog->show();
secondKeySequenceEdit->setKeySequence(QKeySequence(qsl("Ctrl+K")));
secondKeySequenceEdit->setFocus();
QCoreApplication::processEvents();
// it focus-proxies to an inner line edit, so ask the wrapper itself
QVERIFY2(secondKeySequenceEdit->hasFocus(), "Shortcut editor did not take the focus");
QSignalSpy secondSpy(secondKeySequenceEdit, &QKeySequenceEdit::editingFinished);
delete keySequenceDialog;
QVERIFY2(secondSpy.count() == 1,
"A focused QKeySequenceEdit no longer reports editingFinished() "
"when its window is destroyed");
}
// #9574: the reported crash - the profile name field is connected to
// slot_saveName() and the dialog is torn down while that field has the focus
void test_connectionDialogDestroyedWithFocusedNameField()
{
// built directly rather than through mudlet::slot_showConnectionDialog()
// so that the profile this test suite loaded does not have the dialog
// connect it straight back and close it
QPointer<dlgConnectionProfiles> dialog = new dlgConnectionProfiles(mudlet::self());
dialog->fillout_form();
dialog->show();
QTest::qWait(100ms);
QVERIFY2(dialog, "Connection dialog closed itself");
// pick our own profile, so that the name field is editing something whose
// renaming can be checked for afterwards
const auto items = dialog->findData(*dialog->listWidget_profiles, mProfileName, dlgConnectionProfiles::csmNameRole);
QVERIFY2(!items.isEmpty(), "Test profile is not listed in the dialog");
dialog->listWidget_profiles->setCurrentItem(items.first());
QTest::qWait(100ms);
const QString renamedTo = qsl("DialogTeardown-Renamed");
focusWithText(dialog->profile_name_entry, renamedTo);
delete dialog;
QVERIFY2(dialog.isNull(), "Connection dialog should have been destroyed");
// slot_saveName() renames the profile's directory, so it running on the way
// down leaves a trace even in a build where the assert is compiled out
QVERIFY2(!QDir(mudlet::getMudletPath(enums::profileHomePath, renamedTo)).exists(), "Being destroyed made the dialog rename the profile");
QVERIFY2(QDir(mudlet::getMudletPath(enums::profileHomePath, mProfileName)).exists(), "The profile lost its directory while the dialog was destroyed");
}
// The same exposure through the preferences' chat name field, which is
// connected to slot_mmcpChatNameChanged()
void test_preferencesDestroyedWithFocusedChatNameField()
{
mudlet::self()->showOptionsDialog(qsl("tab_chat"), mpHost);
QTest::qWait(100ms);
auto* preferences = mpHost->mpDlgProfilePreferences.data();
QVERIFY2(preferences, "Preferences dialog was not created");
const QString chatNameBefore = mpHost->getMMCPChatName();
const QString typedChatName = qsl("DialogTeardownChatName");
QVERIFY2(chatNameBefore != typedChatName, "Test needs to type a chat name that is not the current one");
focusWithText(preferences->lineEdit_mmcpChatName, typedChatName);
delete preferences;
QVERIFY2(mpHost->mpDlgProfilePreferences.isNull(), "Preferences dialog should have been destroyed");
QCOMPARE(mpHost->getMMCPChatName(), chatNameBefore);
}
// Opening the preferences at all used to be enough to end the run: the
// dialog asks the updater whether it downloads updates by itself, which on
// macOS reaches into Sparkle - and Sparkle is only created by
// checkUpdatesOnStart(), which no test calls. Development builds skip that
// whole branch, so only PTB and release builds ever crashed and CI stayed
// green until the nightly PTB. DEV_UPDATER puts this build on the same path.
void test_preferencesOpensBeforeTheUpdaterIsSetUp()
{
qputenv("DEV_UPDATER", "1");
auto restoreEnvironment = qScopeGuard([]() {
qunsetenv("DEV_UPDATER");
});
mudlet::self()->showOptionsDialog(qsl("tab_specialOptions"), mpHost);
QTest::qWait(100ms);
auto* preferences = mpHost->mpDlgProfilePreferences.data();
QVERIFY2(preferences, "Preferences dialog was not created");
#if defined(INCLUDE_UPDATER)
auto* updater = mudlet::self()->pUpdater;
QVERIFY2(updater, "An updater-enabled build has no updater");
// the dev-build branch disables the checkbox and touches no updater, so
// this is what says the test is on the crashing path at all
QVERIFY2(preferences->checkbox_noAutomaticUpdates->isEnabled(), "DEV_UPDATER no longer moves a development build onto the release update path - this test covers nothing now");
// isHidden() rather than isVisible(): the group box sits on a tab page,
// and only an explicit hide() should count here
QCOMPARE(preferences->groupBox_updates->isHidden(), !updater->ready());
if (!updater->ready()) {
// the accessors the dialog and the Help menu reach for have to be
// safe to call in this state, not merely avoidable
QVERIFY2(!updater->updateAutomatically(), "An updater with no platform updater claimed it auto-updates");
updater->setAutomaticUpdates(true);
updater->manuallyCheckUpdates();
QVERIFY2(!updater->updateAutomatically(), "An updater with no platform updater took a setting it cannot store");
}
#endif
delete preferences;
QVERIFY2(mpHost->mpDlgProfilePreferences.isNull(), "Preferences dialog should have been destroyed");
}
infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
// ...and through the editor, where the item name field is connected to
// slot_saveProperty_TriggerName(). The editor is a QMainWindow rather than a
// QDialog, which makes no difference: it hides itself on the way down too
void test_triggerEditorDestroyedWithFocusedNameField()
{
mudlet::self()->slot_showScriptDialog();
QTest::qWait(100ms);
auto* editor = mpHost->mpEditorDialog.data();
QVERIFY2(editor, "Editor was not created");
// the item fields only appear once an item is being edited
editor->slot_showTriggers();
editor->slot_addNewItem();
QTest::qWait(100ms);
auto* nameField = editor->findChild<QLineEdit*>(qsl("lineEdit_trigger_name"));
QVERIFY2(nameField, "Trigger name field not found in the editor");
const QString nameBefore = nameField->text();
QVERIFY2(mpHost->getTriggerUnit()->findTrigger(nameBefore), "The new trigger is not registered under the name in the field");
const QString typedName = qsl("DialogTeardown trigger");
focusWithText(nameField, typedName);
delete editor;
QVERIFY2(mpHost->mpEditorDialog.isNull(), "Editor should have been destroyed");
// slot_saveProperty_TriggerName() renames the trigger itself, so the item
// shows whether it ran while the editor was being destroyed
QVERIFY2(!mpHost->getTriggerUnit()->findTrigger(typedName), "Being destroyed made the editor rename the trigger");
QVERIFY2(mpHost->getTriggerUnit()->findTrigger(nameBefore), "The trigger lost its name while the editor was destroyed");
}
void test_protocolActionsFireAfterPreferencesReopen()
{
mudlet::self()->showOptionsDialog(qsl("tab_general"), mpHost);
QTest::qWait(100ms);
auto* first = mpHost->mpDlgProfilePreferences.data();
QVERIFY2(first, "Preferences dialog was not created");
delete first;
QVERIFY2(mpHost->mpDlgProfilePreferences.isNull(), "Preferences dialog should have been destroyed");
mudlet::self()->showOptionsDialog(qsl("tab_general"), mpHost);
QTest::qWait(100ms);
auto* preferences = mpHost->mpDlgProfilePreferences.data();
QVERIFY2(preferences, "Preferences dialog was not recreated");
QAction* gmcpAction = nullptr;
for (auto* action : preferences->findChildren<QAction*>()) {
if (action->text().startsWith(qsl("GMCP"))) {
gmcpAction = action;
break;
}
}
QVERIFY2(gmcpAction, "GMCP protocol action not found under the reopened dialog - parenting to the menu broke discovery or population");
// initWithHost() wires GMCP's toggled() to this button's setEnabled(),
// so the button flipping proves the fresh action is connected
const bool enabledBefore = preferences->pushButton_forgetSavedSignIn->isEnabled();
QCOMPARE(enabledBefore, gmcpAction->isChecked());
gmcpAction->toggle();
QCOMPARE(preferences->pushButton_forgetSavedSignIn->isEnabled(), !enabledBefore);
gmcpAction->toggle();
QCOMPARE(preferences->pushButton_forgetSavedSignIn->isEnabled(), enabledBefore);
delete preferences;
}
infrastructure: make windows safe to destroy while a field has the text cursor (#9596) #### Brief overview of PR changes/additions - New `utils::disconnectChildSignals()`, called by the destructors of the connection dialog, the preferences and the editor: a window stops listening to its own widgets before it goes away. - Covers the reported case (connection dialog `Profile name` field) plus the same exposure found in the preferences (MMCP chat name, shortcut editors) and the editor (item name, command, pattern and sound file fields). The preferences and the editor had no destructor at all before this. - New `DialogTeardownTest` covering all three windows, plus a canary that fails if a future Qt stops emitting the focus-out signals the whole thing rests on. #### Motivation for adding to Mudlet Destroying one of these windows while the text cursor sits in one of its fields aborts the run - which is how #9574 turned up, in a functional test - and the windows should simply be safe to destroy, rather than safe only along the `close()` paths that happen to hide them first. #### Other info (issues closed, discussion etc) Closes #9574. The mechanism: a visible window is taken off the screen while its base-class destructors unwind (`~QDialog` hides it, `~QWidget` closes any other window class). That moves the keyboard focus off the field holding it, the field reports `editingFinished()`, and Qt delivers that to a slot of an object whose derived part is already gone: ``` ASSERT failure in dlgConnectionProfiles: "Called object is not of the correct type (class destructor may have already run)" ``` **How much of this can a player hit today: as far as I can trace, none of it**, which is why there are no crash reports behind this: - Every production teardown goes through `close()` / `accept()` / `reject()` first, and that hide happens while the object is still whole - so the field's `editingFinished()` is delivered normally and the edit is saved, exactly as before. `Host::closeChildren()` closes the editor that way, `mudlet::closeEvent()` closes the connection dialog that way. - Nothing `delete`s or `deleteLater()`s these three windows directly. - At exit `main()` deletes the QApplication, which destroys platform windows without running widget destructors, so the preferences dialog - the one window nothing explicitly closes - is never destructed either. - The assert is a `Q_ASSERT_X`, and since we never set `CMAKE_BUILD_TYPE`, Qt defines `QT_NO_DEBUG` for our builds and compiles it out. A shipped build would not abort at that point; it would run the slot against destroyed members instead, which is undefined behaviour that can quietly rename a profile or a trigger. So this is a latent trap rather than a live player crash: it fires today in the test suite, and it fires the moment any future code destroys one of these windows while it is on screen. The fix is small enough to be worth taking on those terms. Verified with standalone Qt probes: `QLineEdit` emits once anything has written to it (`setText()` is enough, even with an empty string), `QAbstractSpinBox` and `QKeySequenceEdit` emit unconditionally, and plain child widgets such as the editor's `dlg*MainArea` panels are not exposed - their slots still run while they are alive. `test/functional_tests/DialogTeardownTest.cpp` is formatted with the repo's clang-format, which the older tests next to it predate. **Test case:** `ctest -R DialogTeardownTest`. All three cases abort on `development` with the assert above and pass here. There is no manual GUI reproduction - see the tracing above.
2026-08-03 11:40:11 +02:00
};
void initializeQRCResourcesForDialogTeardownTest()
{
#ifdef INCLUDE_VARIABLE_SPLASH_SCREEN
qInitResources_additional_splash_screens();
#endif
#ifdef INCLUDE_FONTS
qInitResources_mudlet_fonts_common();
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
qInitResources_mudlet_fonts_posix();
#endif
#endif
qInitResources_mudlet();
qInitResources_qm();
}
#include "DialogTeardownTest.moc"
QTEST_MAIN(DialogTeardownTest)