From 851143eff14dd732fea6be10b96c0bfc798cd629 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Tue, 8 Feb 2022 16:37:29 +0100 Subject: [PATCH] macOS: Prevent recursion when modifying the edit menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amends d42cfeb84faf154b46f2811b2059946b396fcc12, which would result in infinite recursion when the Edit menu was populated and added to the menubar after the menubar has been shown. Add a macOS-only test case that reproduces the crash without the fix. Fixes: QTBUG-100441 Pick-to: 6.3 Change-Id: I018a7aa7f01558a3b9732b4d6d96a911dc7fbd19 Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qcocoamenubar.mm | 9 +++++- .../widgets/widgets/qmenubar/tst_qmenubar.cpp | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm index 365346ac8cd..aaea688b394 100644 --- a/src/plugins/platforms/cocoa/qcocoamenubar.mm +++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm @@ -196,8 +196,15 @@ void QCocoaMenuBar::syncMenu_helper(QPlatformMenu *menu, bool menubarUpdate) const QString captionNoAmpersand = QString::fromNSString(cocoaMenu->nsMenu().title) .remove(QLatin1Char('&')); - if (captionNoAmpersand == QCoreApplication::translate("QCocoaMenu", "Edit")) + if (captionNoAmpersand == QCoreApplication::translate("QCocoaMenu", "Edit")) { + // prevent recursion from QCocoaMenu::insertMenuItem - when the menu is visible + // it calls syncMenu again. QCocoaMenu::setVisible just sets the bool, which then + // gets evaluated in the code after this block. + const bool wasVisible = cocoaMenu->isVisible(); + cocoaMenu->setVisible(false); insertDefaultEditItems(cocoaMenu); + cocoaMenu->setVisible(wasVisible); + } BOOL shouldHide = YES; if (cocoaMenu->isVisible()) { diff --git a/tests/auto/widgets/widgets/qmenubar/tst_qmenubar.cpp b/tests/auto/widgets/widgets/qmenubar/tst_qmenubar.cpp index 1b8de0b75ca..ae08c7ae984 100644 --- a/tests/auto/widgets/widgets/qmenubar/tst_qmenubar.cpp +++ b/tests/auto/widgets/widgets/qmenubar/tst_qmenubar.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include @@ -144,6 +145,8 @@ private slots: #ifdef Q_OS_MACOS void taskQTBUG56275_reinsertMenuInParentlessQMenuBar(); void QTBUG_57404_existingMenuItemException(); + void defaultEditMenuItems(); + #endif void QTBUG_25669_menubarActionDoubleTriggered(); void taskQTBUG55966_subMenuRemoved(); @@ -1818,6 +1821,32 @@ void tst_QMenuBar::QTBUG_57404_existingMenuItemException() QTest::qWait(100); // No crash, all fine. Ideally, there should be only one warning. } + +void tst_QMenuBar::defaultEditMenuItems() +{ + class TestTranslator : public QTranslator + { + public: + QString translate(const char *context, const char *sourceText, + const char *disambiguation = nullptr, int n = -1) const override + { + if (QByteArrayView(context) == "QCocoaMenu" && QByteArrayView(sourceText) == "Edit") + return QString("Editieren"); + return QTranslator::translate(context, sourceText, disambiguation, n); + } + } testTranslator; + qApp->installTranslator(&testTranslator); + + QMainWindow mw; + mw.show(); + QVERIFY(QTest::qWaitForWindowActive(&mw)); + + mw.menuBar()->addMenu("Editieren")->addAction("Undo"); + + mw.hide(); + mw.show(); + // this should not crash with infinite recursion +} #endif // Q_OS_MACOS void tst_QMenuBar::taskQTBUG55966_subMenuRemoved()