QCocoaMenu: Avoid exception when inserting item already in this menu
This should not happen, but it's clearly not the user's fault. So we should try to carry on as gracefully as possible instead of letting Cocoa abort the application. The patch also factors the repeated calls to QCocoaMenuItem:: nsItem() in QCocoaMenu::insertNative() and improves a warning from QCocoaMenuIten::sync(). Change-Id: Id00135c219aaf40fb565b19a65cab68f6d9863b2 Task-number: QTBUG-57404 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
parent
4077fcc615
commit
b2f78b796b
@ -331,11 +331,12 @@ void QCocoaMenu::insertMenuItem(QPlatformMenuItem *menuItem, QPlatformMenuItem *
|
||||
|
||||
void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
|
||||
{
|
||||
item->nsItem().target = m_nativeMenu.delegate;
|
||||
item->nsItem().action = @selector(itemFired:);
|
||||
NSMenuItem *nativeItem = item->nsItem();
|
||||
nativeItem.target = m_nativeMenu.delegate;
|
||||
nativeItem.action = @selector(itemFired:);
|
||||
// Someone's adding new items after aboutToShow() was emitted
|
||||
if (isOpen() && item->menu() && item->nsItem())
|
||||
item->menu()->setAttachedItem(item->nsItem());
|
||||
if (isOpen() && nativeItem)
|
||||
item->menu()->setAttachedItem(nativeItem);
|
||||
|
||||
item->setParentEnabled(isEnabled());
|
||||
|
||||
@ -348,15 +349,20 @@ void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
|
||||
beforeItem = itemOrNull(m_menuItems.indexOf(beforeItem) + 1);
|
||||
}
|
||||
|
||||
if (nativeItem.menu) {
|
||||
qWarning() << "Menu item" << item->text() << "already in menu" << QString::fromNSString(nativeItem.menu.title);
|
||||
return;
|
||||
}
|
||||
|
||||
if (beforeItem) {
|
||||
if (beforeItem->isMerged()) {
|
||||
qWarning("No non-merged before menu item found");
|
||||
return;
|
||||
}
|
||||
NSUInteger nativeIndex = [m_nativeMenu indexOfItem:beforeItem->nsItem()];
|
||||
[m_nativeMenu insertItem: item->nsItem() atIndex: nativeIndex];
|
||||
const NSInteger nativeIndex = [m_nativeMenu indexOfItem:beforeItem->nsItem()];
|
||||
[m_nativeMenu insertItem:nativeItem atIndex:nativeIndex];
|
||||
} else {
|
||||
[m_nativeMenu addItem: item->nsItem()];
|
||||
[m_nativeMenu addItem:nativeItem];
|
||||
}
|
||||
item->setMenuParent(this);
|
||||
}
|
||||
|
@ -288,7 +288,7 @@ NSMenuItem *QCocoaMenuItem::sync()
|
||||
}
|
||||
|
||||
default:
|
||||
qWarning() << "menu item" << m_text << "has unsupported role" << (int)m_role;
|
||||
qWarning() << "Menu item" << m_text << "has unsupported role" << m_role;
|
||||
}
|
||||
|
||||
if (mergeItem) {
|
||||
|
@ -132,6 +132,7 @@ private slots:
|
||||
void taskQTBUG53205_crashReparentNested();
|
||||
#ifdef Q_OS_MACOS
|
||||
void taskQTBUG56275_reinsertMenuInParentlessQMenuBar();
|
||||
void QTBUG_57404_existingMenuItemException();
|
||||
#endif
|
||||
|
||||
void platformMenu();
|
||||
@ -1539,6 +1540,32 @@ void tst_QMenuBar::taskQTBUG56275_reinsertMenuInParentlessQMenuBar()
|
||||
|
||||
QVERIFY(tst_qmenubar_taskQTBUG56275(&menubar));
|
||||
}
|
||||
|
||||
void tst_QMenuBar::QTBUG_57404_existingMenuItemException()
|
||||
{
|
||||
QMainWindow mw1;
|
||||
QMainWindow mw2;
|
||||
mw1.show();
|
||||
mw2.show();
|
||||
|
||||
QMenuBar *mb = new QMenuBar(&mw1);
|
||||
mw1.setMenuBar(mb);
|
||||
mb->show();
|
||||
QMenu *editMenu = new QMenu(QLatin1String("Edit"), &mw1);
|
||||
mb->addMenu(editMenu);
|
||||
QAction *copyAction = editMenu->addAction("&Copy");
|
||||
copyAction->setShortcut(QKeySequence("Ctrl+C"));
|
||||
QTest::ignoreMessage(QtWarningMsg, "Menu item \"&Copy\" has unsupported role QPlatformMenuItem::MenuRole(NoRole)");
|
||||
copyAction->setMenuRole(QAction::NoRole);
|
||||
|
||||
QVERIFY(QTest::qWaitForWindowExposed(&mw2));
|
||||
QTest::qWait(100);
|
||||
QTest::ignoreMessage(QtWarningMsg, "Menu item \"&Copy\" already in menu \"Edit\"");
|
||||
mw2.close();
|
||||
mw1.activateWindow();
|
||||
QTest::qWait(100);
|
||||
// No crash, all fine.
|
||||
}
|
||||
#endif // Q_OS_MACOS
|
||||
|
||||
QTEST_MAIN(tst_QMenuBar)
|
||||
|
Loading…
Reference in New Issue
Block a user