[ksnapshot was indeed fixed rather long ago for this, but let's discuss the
kdialog behavior]
On Thursday 02 December 2010, Aaron J. Seigo wrote:
> it's KDialog that making the assertion; and the assertion looks bogus since
> it's absolutely plausible that the app requests the default to be a button
> that doesn't actually exist.
The point is that making a non-existing button default never worked. But
before it would just silently break, while now it asserts.
(and the assert says "defaultButton will return the value set by
setDefaultButton", it can't possibly be a wrong assert, it just revealed the
issue of calling the method with a non-existing button).
I guess your point is "KDialog should remember and make the button default
once it exists". Hmm. It can be done indeed. I don't know if it's worth the
additional complexity, but if you feel this should work, please review the
attached patch. Unittest passes, of course.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Index: dialogs/kdialog_p.h
===================================================================
--- dialogs/kdialog_p.h (revision 1201251)
+++ dialogs/kdialog_p.h (working copy)
@@ -41,6 +41,7 @@
mDetailsWidget(0),
mTopLayout(0), mMainWidget(0), mUrlHelp(0), mActionSeparator(0),
mButtonOrientation(Qt::Horizontal),
+ mDefaultButton(KDialog::NoDefault),
mButtonBox(0)
{
}
@@ -72,6 +73,7 @@
QString mHelpLinkText;
Qt::Orientation mButtonOrientation;
+ KDialog::ButtonCode mDefaultButton;
KDialog::ButtonCode mEscapeButton;
QDialogButtonBox *mButtonBox;
Index: dialogs/kdialog.cpp
===================================================================
--- dialogs/kdialog.cpp (revision 1201251)
+++ dialogs/kdialog.cpp (working copy)
@@ -110,6 +110,8 @@
void KDialogPrivate::appendButton(KDialog::ButtonCode key, const KGuiItem &item)
{
+ Q_Q(KDialog);
+
QDialogButtonBox::ButtonRole role = QDialogButtonBox::InvalidRole;
switch ( key ) {
case KDialog::Help:
@@ -158,6 +160,11 @@
QObject::connect(button, SIGNAL(clicked()),
&mButtonSignalMapper, SLOT( map() ) );
+
+ if (key == mDefaultButton) {
+ // Now that it exists, set it as default
+ q->setDefaultButton(mDefaultButton);
+ }
}
void KDialogPrivate::init(KDialog *q)
@@ -279,6 +286,8 @@
void KDialog::setDefaultButton( ButtonCode newDefaultButton )
{
+ Q_D(KDialog);
+
if (newDefaultButton == None)
newDefaultButton = NoDefault; // #148969
@@ -308,6 +317,7 @@
}
}
}
+ d->mDefaultButton = newDefaultButton;
Q_ASSERT(defaultButton() == newDefaultButton);
}
@@ -322,7 +332,7 @@
}
}
- return NoDefault;
+ return d->mDefaultButton;
}
void KDialog::setMainWidget( QWidget *widget )
Index: tests/kdialog_unittest.cpp
===================================================================
--- tests/kdialog_unittest.cpp (revision 1201251)
+++ tests/kdialog_unittest.cpp (working copy)
@@ -77,6 +77,22 @@
checkOtherButtonsAreNotDefault(dialog, KDialog::None);
}
+ void testFutureDefaultButton()
+ {
+ // Test setting a default button that doesn't exist yet
+ KDialog dialog;
+ KDialog::ButtonCode id = KDialog::Apply;
+ dialog.setDefaultButton(id);
+ QCOMPARE(dialog.defaultButton(), id);
+ QCOMPARE(dialog.button(id), static_cast<KPushButton *>(0));
+
+ dialog.setButtons(KDialog::Ok | KDialog::Apply
+ | KDialog::Cancel | KDialog::No | KDialog::Yes);
+ QCOMPARE(dialog.defaultButton(), KDialog::Apply);
+ QVERIFY(dialog.button(id)->isDefault());
+ QVERIFY(!dialog.button(KDialog::Ok)->isDefault());
+ }
+
// Test what happens with giving focus to widgets before the window is shown
// This is mostly Qt experimentation, unrelated to KDialog's own code
void testFocus()
