From 0422205803c8f6980a447fdaa1dc270a486970cd Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 16:21:42 +0200 Subject: fix(rules): save the window size on Cancel and Save, not only on X The geometry was saved from closeEvent, and neither dialog button sends one: Cancel calls reject(), Save calls accept(), and only the window manager's X button produces a QCloseEvent. So the size and the column widths were kept for the one route out of three that a user almost never takes, and a resize followed by Cancel came back forgotten. The save moves to a done(int) override, which both buttons funnel through and which QWidget::close() also reaches. The test that covered this passed against the bug because it asserted with close(). It now drives all three routes rather than trusting one to stand for the others, and shows the dialog before the close leg: close() on a widget that was never visible returns early without reaching done(), so that assertion would otherwise prove nothing. Both traps recorded in CLAUDE.md, since neither is specific to this dialog. Co-Authored-By: Claude Opus 5 --- src/tagrulesdialog.cpp | 20 ++++++++++++++------ src/tagrulesdialog.h | 7 +++++-- 2 files changed, 19 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp index 61f2c21..54600c3 100644 --- a/src/tagrulesdialog.cpp +++ b/src/tagrulesdialog.cpp @@ -20,7 +20,6 @@ #include #include -#include #include #include #include @@ -297,13 +296,22 @@ void TagRulesDialog::saveUiState() m_list->header()->saveState()); } -/// Saves on close rather than on accept, so a size the user chose is kept -/// whether they pressed Save or Cancel. The window's shape is not part of the -/// edit being confirmed. -void TagRulesDialog::closeEvent(QCloseEvent *event) +/// Saves on the way out, whichever way that is. +/// +/// done() rather than closeEvent, and this distinction shipped broken: Cancel +/// calls reject() and Save calls accept(), and NEITHER sends a QCloseEvent. +/// Only the window manager's X button does. Saving from closeEvent therefore +/// kept the size for the one route the buttons never take, which is how a +/// resize followed by Cancel came back forgotten. Both buttons funnel through +/// done(), and QWidget::close() reaches it too. +/// +/// On every route, not only on accept: the window's shape is not part of the +/// edit being confirmed, so Cancel should discard the rule changes and keep +/// the size. +void TagRulesDialog::done(int result) { saveUiState(); - QDialog::closeEvent(event); + QDialog::done(result); } int TagRulesDialog::columnWidthForTest(int column) const diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h index f86d210..bdcaff9 100644 --- a/src/tagrulesdialog.h +++ b/src/tagrulesdialog.h @@ -123,8 +123,11 @@ private slots: void applyEditsToCurrentRule(); void onSave(); -protected: - void closeEvent(QCloseEvent *event) override; +public slots: + /// Saves the window's size and column widths on the way out. Overridden + /// here rather than closeEvent because Save and Cancel do not send a + /// close event at all, only the window manager's X button does. + void done(int result) override; private: void reloadList(); -- cgit v1.2.3