Skip to content

Commit

Permalink
QMenuBar: Update title on change
Browse files Browse the repository at this point in the history
When one of the menubar actions changed, we would omit to
update several properties on the platform menu, most notably
its title.

Manual tested with BigMenuCreator, where the sequence

    menu->addAction(action);    // A-operation
    action->setMenu(submenu);   // S-operation

would result in an "Untitled" menubar item on macOS, and this
regardless of when the submenu is populated.

Change-Id: I43989f36f6bf3f0b7056310ac986c06f8e02f128
Reviewed-by: Dmitry Shachnev <mitya57@gmail.com>
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
  • Loading branch information
Gabriel de Dietrich authored and ec1oud committed Nov 7, 2017
1 parent 237b1c1 commit 7986e1e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 24 deletions.
54 changes: 31 additions & 23 deletions src/widgets/widgets/qmenubar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ void QMenuBar::leaveEvent(QEvent *)
d->setCurrentAction(0);
}

QPlatformMenu *QMenuBarPrivate::getPlatformMenu(QAction *action)
QPlatformMenu *QMenuBarPrivate::getPlatformMenu(const QAction *action)
{
if (!action || !action->menu())
return 0;
Expand All @@ -1202,6 +1202,29 @@ QPlatformMenu *QMenuBarPrivate::getPlatformMenu(QAction *action)
return platformMenu;
}

QPlatformMenu *QMenuBarPrivate::findInsertionPlatformMenu(const QAction *action)
{
Q_Q(QMenuBar);
QPlatformMenu *beforeMenu = nullptr;
for (int beforeIndex = indexOf(const_cast<QAction *>(action)) + 1;
!beforeMenu && (beforeIndex < q->actions().size());
++beforeIndex) {
beforeMenu = getPlatformMenu(q->actions().at(beforeIndex));
}

return beforeMenu;
}

void QMenuBarPrivate::copyActionToPlatformMenu(const QAction *action, QPlatformMenu *menu)
{
const auto tag = reinterpret_cast<quintptr>(action);
if (menu->tag() != tag)
menu->setTag(tag);
menu->setText(action->text());
menu->setVisible(action->isVisible());
menu->setEnabled(action->isEnabled());
}

/*!
\reimp
*/
Expand All @@ -1218,24 +1241,17 @@ void QMenuBar::actionEvent(QActionEvent *e)
if (e->type() == QEvent::ActionAdded) {
QPlatformMenu *menu = d->getPlatformMenu(e->action());
if (menu) {
QPlatformMenu* beforeMenu = NULL;
for (int beforeIndex = d->indexOf(e->action()) + 1;
!beforeMenu && (beforeIndex < actions().size());
++beforeIndex)
{
beforeMenu = d->getPlatformMenu(actions().at(beforeIndex));
}
d->copyActionToPlatformMenu(e->action(), menu);

menu->setTag(reinterpret_cast<quintptr>(e->action()));
menu->setText(e->action()->text());
QPlatformMenu *beforeMenu = d->findInsertionPlatformMenu(e->action());
d->platformMenuBar->insertMenu(menu, beforeMenu);
}
} else if (e->type() == QEvent::ActionRemoved) {
QPlatformMenu *menu = d->getPlatformMenu(e->action());
if (menu)
d->platformMenuBar->removeMenu(menu);
} else if (e->type() == QEvent::ActionChanged) {
QPlatformMenu* cur = d->platformMenuBar->menuForTag(reinterpret_cast<quintptr>(e->action()));
QPlatformMenu *cur = d->platformMenuBar->menuForTag(reinterpret_cast<quintptr>(e->action()));
QPlatformMenu *menu = d->getPlatformMenu(e->action());

// the menu associated with the action can change, need to
Expand All @@ -1244,21 +1260,13 @@ void QMenuBar::actionEvent(QActionEvent *e)
if (cur)
d->platformMenuBar->removeMenu(cur);
if (menu) {
menu->setTag(reinterpret_cast<quintptr>(e->action()));

QPlatformMenu* beforeMenu = NULL;
for (int beforeIndex = d->indexOf(e->action()) + 1;
!beforeMenu && (beforeIndex < actions().size());
++beforeIndex)
{
beforeMenu = d->getPlatformMenu(actions().at(beforeIndex));
}
d->copyActionToPlatformMenu(e->action(), menu);

QPlatformMenu *beforeMenu = d->findInsertionPlatformMenu(e->action());
d->platformMenuBar->insertMenu(menu, beforeMenu);
}
} else if (menu) {
menu->setText(e->action()->text());
menu->setVisible(e->action()->isVisible());
menu->setEnabled(e->action()->isEnabled());
d->copyActionToPlatformMenu(e->action(), menu);
d->platformMenuBar->syncMenu(menu);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/widgets/widgets/qmenubar_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ class QMenuBarPrivate : public QWidgetPrivate

QBasicTimer autoReleaseTimer;
QPlatformMenuBar *platformMenuBar;
QPlatformMenu *getPlatformMenu(QAction *action);
QPlatformMenu *getPlatformMenu(const QAction *action);
QPlatformMenu *findInsertionPlatformMenu(const QAction *action);
void copyActionToPlatformMenu(const QAction *e, QPlatformMenu *menu);

inline int indexOf(QAction *act) const { return q_func()->actions().indexOf(act); }
};
Expand Down

0 comments on commit 7986e1e

Please sign in to comment.