-
Notifications
You must be signed in to change notification settings - Fork 669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make 'No' button default in Unknown Certificate dialog #11540
Conversation
The ui tests have multiple crashes in the tls dialog, please have a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^
3e35089
to
f0c7f3e
Compare
src/gui/tlserrordialog.cpp
Outdated
|
||
// Try to set the No-button (the reject button) as the default | ||
if (QPushButton *noButton = _ui->buttonBox->button(QDialogButtonBox::No)) { | ||
Q_ASSERT(_ui->buttonBox->buttonRole(noButton) == QDialogButtonBox::RejectRole || _ui->buttonBox->buttonRole(noButton) == QDialogButtonBox::NoRole); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm according to https://doc.qt.io/qt-6/qdialogbuttonbox.html#ButtonRole-enum it is supposed to always be NoRole
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But how can clicking the "No" button then reject the dialog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, in QDialogButtonBoxPrivate::_q_handleButtonClicked()
:
case QPlatformDialogHelper::RejectRole:
case QPlatformDialogHelper::NoRole:
emit q->rejected();
break;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the assert, it's superfluous to check the NoRole
on the "No" button.
f0c7f3e
to
a5763ec
Compare
Fixes: #11531