From 7dc27689c7ed887c1f8b7cfa7f3a4e71983e4a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Sun, 22 Oct 2023 17:53:34 +0200 Subject: [PATCH] Teach QMessageDialogOptions about default and escape buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's logic in QMessageBox to resolve the default and escape button if not set by the user. And the user may of course override these. We now propagate the resulting buttons via the dialog helper, and pick them up in the macOS native dialog backend. The only common information we have between the standard buttons and custom buttons is the button identifier, which for standard buttons is the enum value, and for custom buttons is an auto generated identifier. The same identifier is used when reporting the clicked button from the native dialog helper. Fixes: QTBUG-118308 Pick-to: 6.5 Change-Id: I5ca45604b51f0bbf74e56134d7b55bb8911f3563 Reviewed-by: Volker Hilsheimer (cherry picked from commit c5d9e4a7a78b82ed31e5225c169de4718dfe4f05) Reviewed-by: Tor Arne Vestbø --- src/gui/kernel/qplatformdialoghelper.cpp | 27 +++++++++++++ src/gui/kernel/qplatformdialoghelper.h | 7 ++++ .../platforms/cocoa/qcocoamessagedialog.mm | 33 +++++++++++----- src/widgets/dialogs/qmessagebox.cpp | 39 ++++++++++++++++--- 4 files changed, 90 insertions(+), 16 deletions(-) diff --git a/src/gui/kernel/qplatformdialoghelper.cpp b/src/gui/kernel/qplatformdialoghelper.cpp index 7153c90f507..847a841cee2 100644 --- a/src/gui/kernel/qplatformdialoghelper.cpp +++ b/src/gui/kernel/qplatformdialoghelper.cpp @@ -778,6 +778,8 @@ public: QPixmap iconPixmap; QString checkBoxLabel; Qt::CheckState checkBoxState = Qt::Unchecked; + int defaultButtonId = 0; + int escapeButtonId = 0; QMessageDialogOptions::Options options; }; @@ -903,6 +905,11 @@ const QList &QMessageDialogOptions::customB return d->customButtons; } +void QMessageDialogOptions::clearCustomButtons() +{ + d->customButtons.clear(); +} + const QMessageDialogOptions::CustomButton *QMessageDialogOptions::customButton(int id) { const int i = int(d->customButtons.indexOf(CustomButton(id))); @@ -925,6 +932,26 @@ Qt::CheckState QMessageDialogOptions::checkBoxState() const return d->checkBoxState; } +void QMessageDialogOptions::setDefaultButton(int id) +{ + d->defaultButtonId = id; +} + +int QMessageDialogOptions::defaultButton() const +{ + return d->defaultButtonId; +} + +void QMessageDialogOptions::setEscapeButton(int id) +{ + d->escapeButtonId = id; +} + +int QMessageDialogOptions::escapeButton() const +{ + return d->escapeButtonId; +} + void QMessageDialogOptions::setOption(QMessageDialogOptions::Option option, bool on) { if (!(d->options & option) != !on) diff --git a/src/gui/kernel/qplatformdialoghelper.h b/src/gui/kernel/qplatformdialoghelper.h index 161ea067a53..78952f946aa 100644 --- a/src/gui/kernel/qplatformdialoghelper.h +++ b/src/gui/kernel/qplatformdialoghelper.h @@ -462,11 +462,18 @@ public: void removeButton(int id); const QList &customButtons(); const CustomButton *customButton(int id); + void clearCustomButtons(); void setCheckBox(const QString &label, Qt::CheckState state); QString checkBoxLabel() const; Qt::CheckState checkBoxState() const; + void setEscapeButton(int id); + int escapeButton() const; + + void setDefaultButton(int id); + int defaultButton() const; + private: QMessageDialogOptionsPrivate *d; }; diff --git a/src/plugins/platforms/cocoa/qcocoamessagedialog.mm b/src/plugins/platforms/cocoa/qcocoamessagedialog.mm index ebe2c637692..43d767d32b7 100644 --- a/src/plugins/platforms/cocoa/qcocoamessagedialog.mm +++ b/src/plugins/platforms/cocoa/qcocoamessagedialog.mm @@ -127,8 +127,8 @@ bool QCocoaMessageDialog::show(Qt::WindowFlags windowFlags, Qt::WindowModality w break; } - bool defaultButtonAdded = false; - bool cancelButtonAdded = false; + auto defaultButton = options()->defaultButton(); + auto escapeButton = options()->escapeButton(); const auto addButton = [&](auto title, auto tag, auto role) { title = QPlatformTheme::removeMnemonics(title); @@ -138,17 +138,25 @@ bool QCocoaMessageDialog::show(Qt::WindowFlags windowFlags, Qt::WindowModality w // and going toward the left/bottom. By default, the first button has a key equivalent of // Return, any button with a title of "Cancel" has a key equivalent of Escape, and any button // with the title "Don't Save" has a key equivalent of Command-D (but only if it's not the first - // button). Unfortunately QMessageBox does not currently plumb setDefaultButton/setEscapeButton - // through the dialog options, so we can't forward this information directly. The closest we - // can get right now is to use the role to set the button's key equivalent. + // button). If an explicit default or escape button has been set, we respect these, + // and otherwise we fall back to role-based default and escape buttons. - if (role == AcceptRole && !defaultButtonAdded) { + if (!defaultButton && role == AcceptRole) + defaultButton = tag; + + if (tag == defaultButton) button.keyEquivalent = @"\r"; - defaultButtonAdded = true; - } else if (role == RejectRole && !cancelButtonAdded) { + else if ([button.keyEquivalent isEqualToString:@"\r"]) + button.keyEquivalent = @""; + + if (!escapeButton && role == RejectRole) + escapeButton = tag; + + // Don't override default button with escape button, to match AppKit default + if (tag == escapeButton && ![button.keyEquivalent isEqualToString:@"\r"]) button.keyEquivalent = @"\e"; - cancelButtonAdded = true; - } + else if ([button.keyEquivalent isEqualToString:@"\e"]) + button.keyEquivalent = @""; if (@available(macOS 11, *)) button.hasDestructiveAction = role == DestructiveRole; @@ -182,6 +190,11 @@ bool QCocoaMessageDialog::show(Qt::WindowFlags windowFlags, Qt::WindowModality w for (auto customButton : customButtons) addButton(customButton.label, customButton.id, customButton.role); + // If we didn't find a an explicit or implicit default button above + // we restore the AppKit behavior of making the first button default. + if (!defaultButton) + m_alert.buttons.firstObject.keyEquivalent = @"\r"; + if (auto checkBoxLabel = options()->checkBoxLabel(); !checkBoxLabel.isNull()) { checkBoxLabel = QPlatformTheme::removeMnemonics(checkBoxLabel); m_alert.suppressionButton.title = checkBoxLabel.toNSString(); diff --git a/src/widgets/dialogs/qmessagebox.cpp b/src/widgets/dialogs/qmessagebox.cpp index 5d5ba6e117d..25de452cfaa 100644 --- a/src/widgets/dialogs/qmessagebox.cpp +++ b/src/widgets/dialogs/qmessagebox.cpp @@ -877,12 +877,6 @@ void QMessageBox::addButton(QAbstractButton *button, ButtonRole role) } } - // Add button to native dialog helper, unless it's the details button, - // since we don't do any plumbing for the button's action in that case. - if (button != d->detailsButton) { - d->options->addButton(button->text(), - static_cast(role), button); - } d->buttonBox->addButton(button, (QDialogButtonBox::ButtonRole)role); d->customButtonList.append(button); d->autoAddOkButton = false; @@ -2846,7 +2840,40 @@ void QMessageBoxPrivate::helperPrepareShow(QPlatformDialogHelper *) #endif options->setStandardIcon(helperIcon(q->icon())); options->setIconPixmap(q->iconPixmap()); + + // Add standard buttons and resolve default/escape button options->setStandardButtons(helperStandardButtons(q)); + for (int button = QDialogButtonBox::StandardButton::FirstButton; + button <= QDialogButtonBox::StandardButton::LastButton; button <<= 1) { + auto *standardButton = buttonBox->button(QDialogButtonBox::StandardButton(button)); + if (!standardButton) + continue; + + if (standardButton == defaultButton) + options->setDefaultButton(button); + else if (standardButton == detectedEscapeButton) + options->setEscapeButton(button); + } + + // Add custom buttons and resolve default/escape button + options->clearCustomButtons(); + for (auto *customButton : customButtonList) { + // Unless it's the details button, since we don't do any + // plumbing for the button's action in that case. + if (customButton == detailsButton) + continue; + + const auto buttonRole = buttonBox->buttonRole(customButton); + const int buttonId = options->addButton(customButton->text(), + static_cast(buttonRole), + customButton); + + if (customButton == defaultButton) + options->setDefaultButton(buttonId); + else if (customButton == detectedEscapeButton) + options->setEscapeButton(buttonId); + } + if (checkbox) options->setCheckBox(checkbox->text(), checkbox->checkState()); }