Cocoa QPA Menus: Propagate enabled state downwards
NSMenu has autoenableItems set to true by default, and we keep it this way in Qt. This means that NSMenuItem's enabled property is basically ignored and therefore QCocoaMenuItem::syncModalState() is wrong. What is also wrong, is syncModalState()'s name in both QCocoaMenuItem and QCocoaMenu. Indeed, this function's role should be to ensure that the enabled state is properly propagated down the menu hierarchy, whether the reason is being in the context of a modal dialog or the parent menu having been disabled by the app. Notice that the latter case is specially needed when a menubar menu is explicitly disabled. Therefore, we introduce a separate flag for the parent enabled state in order to avoid polluting the app-set enabled state flag. This is done in both QCocoaMenu and QCocoaMenuItem. In the case of QCocoaMenuItem, these two flags define whether an NSMenuItem is enabled state conjointly, and set from -[QCocoaMenuDelegate validateMenuItem:]. The rest of the logic remains as before. Similar logic is used in QCocoaMenu::isEnabled(). In addition, the presence of the second flag allows us to show disabled submenus in the same fashion native Cocoa applications do. This means, the submenu item itself remains enabled, allowing to show the submenu popup where all its menu items will appear disabled. Bonus change: merged all the bool flags into a bitfield and made the compiler happy about the ivar reordering in QCocoaMenu and QCocoaMenuItem's constructor. Task-number: QTBUG-54698 Task-number: QTBUG-55121 Change-Id: Ie156cb3aa57a519103908ad4605f7b43c57e5aef Reviewed-by: Timur Pocheptsov <timur.pocheptsov@theqtcompany.com>
This commit is contained in:
parent
810363945f
commit
9b491616f8
src/plugins/platforms/cocoa
@ -65,7 +65,7 @@ public:
|
|||||||
|
|
||||||
void syncSeparatorsCollapsible(bool enable) Q_DECL_OVERRIDE;
|
void syncSeparatorsCollapsible(bool enable) Q_DECL_OVERRIDE;
|
||||||
|
|
||||||
void syncModalState(bool modal);
|
void propagateEnabledState(bool enabled);
|
||||||
|
|
||||||
void setIcon(const QIcon &icon) Q_DECL_OVERRIDE { Q_UNUSED(icon) }
|
void setIcon(const QIcon &icon) Q_DECL_OVERRIDE { Q_UNUSED(icon) }
|
||||||
|
|
||||||
@ -98,9 +98,10 @@ private:
|
|||||||
NSMenu *m_nativeMenu;
|
NSMenu *m_nativeMenu;
|
||||||
NSMenuItem *m_attachedItem;
|
NSMenuItem *m_attachedItem;
|
||||||
quintptr m_tag;
|
quintptr m_tag;
|
||||||
bool m_enabled;
|
bool m_enabled:1;
|
||||||
bool m_visible;
|
bool m_parentEnabled:1;
|
||||||
bool m_isOpen;
|
bool m_visible:1;
|
||||||
|
bool m_isOpen:1;
|
||||||
};
|
};
|
||||||
|
|
||||||
QT_END_NAMESPACE
|
QT_END_NAMESPACE
|
||||||
|
@ -154,10 +154,10 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaMenuDelegate);
|
|||||||
|
|
||||||
- (BOOL)validateMenuItem:(NSMenuItem*)menuItem
|
- (BOOL)validateMenuItem:(NSMenuItem*)menuItem
|
||||||
{
|
{
|
||||||
if (![menuItem tag])
|
QCocoaMenuItem *cocoaItem = reinterpret_cast<QCocoaMenuItem *>(menuItem.tag);
|
||||||
|
if (!cocoaItem)
|
||||||
return YES;
|
return YES;
|
||||||
|
|
||||||
QCocoaMenuItem* cocoaItem = reinterpret_cast<QCocoaMenuItem *>([menuItem tag]);
|
|
||||||
return cocoaItem->isEnabled();
|
return cocoaItem->isEnabled();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -255,6 +255,7 @@ QCocoaMenu::QCocoaMenu() :
|
|||||||
m_attachedItem(0),
|
m_attachedItem(0),
|
||||||
m_tag(0),
|
m_tag(0),
|
||||||
m_enabled(true),
|
m_enabled(true),
|
||||||
|
m_parentEnabled(true),
|
||||||
m_visible(true),
|
m_visible(true),
|
||||||
m_isOpen(false)
|
m_isOpen(false)
|
||||||
{
|
{
|
||||||
@ -330,6 +331,8 @@ void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
|
|||||||
else if (isOpen() && item->nsItem()) // Someone's adding new items after aboutToShow() was emitted
|
else if (isOpen() && item->nsItem()) // Someone's adding new items after aboutToShow() was emitted
|
||||||
item->menu()->setAttachedItem(item->nsItem());
|
item->menu()->setAttachedItem(item->nsItem());
|
||||||
|
|
||||||
|
item->setParentEnabled(isEnabled());
|
||||||
|
|
||||||
if (item->isMerged())
|
if (item->isMerged())
|
||||||
return;
|
return;
|
||||||
|
|
||||||
@ -374,6 +377,9 @@ void QCocoaMenu::removeMenuItem(QPlatformMenuItem *menuItem)
|
|||||||
if (cocoaItem->menuParent() == this)
|
if (cocoaItem->menuParent() == this)
|
||||||
cocoaItem->setMenuParent(0);
|
cocoaItem->setMenuParent(0);
|
||||||
|
|
||||||
|
// Ignore any parent enabled state
|
||||||
|
cocoaItem->setParentEnabled(true);
|
||||||
|
|
||||||
m_menuItems.removeOne(cocoaItem);
|
m_menuItems.removeOne(cocoaItem);
|
||||||
if (!cocoaItem->isMerged()) {
|
if (!cocoaItem->isMerged()) {
|
||||||
if (m_nativeMenu != [cocoaItem->nsItem() menu]) {
|
if (m_nativeMenu != [cocoaItem->nsItem() menu]) {
|
||||||
@ -464,13 +470,17 @@ void QCocoaMenu::syncSeparatorsCollapsible(bool enable)
|
|||||||
|
|
||||||
void QCocoaMenu::setEnabled(bool enabled)
|
void QCocoaMenu::setEnabled(bool enabled)
|
||||||
{
|
{
|
||||||
|
if (m_enabled == enabled)
|
||||||
|
return;
|
||||||
m_enabled = enabled;
|
m_enabled = enabled;
|
||||||
syncModalState(!m_enabled);
|
const bool wasParentEnabled = m_parentEnabled;
|
||||||
|
propagateEnabledState(m_enabled);
|
||||||
|
m_parentEnabled = wasParentEnabled; // Reset to the parent value
|
||||||
}
|
}
|
||||||
|
|
||||||
bool QCocoaMenu::isEnabled() const
|
bool QCocoaMenu::isEnabled() const
|
||||||
{
|
{
|
||||||
return m_attachedItem ? [m_attachedItem isEnabled] : m_enabled;
|
return m_attachedItem ? [m_attachedItem isEnabled] : m_enabled && m_parentEnabled;
|
||||||
}
|
}
|
||||||
|
|
||||||
void QCocoaMenu::setVisible(bool visible)
|
void QCocoaMenu::setVisible(bool visible)
|
||||||
@ -604,20 +614,19 @@ QList<QCocoaMenuItem *> QCocoaMenu::merged() const
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
void QCocoaMenu::syncModalState(bool modal)
|
void QCocoaMenu::propagateEnabledState(bool enabled)
|
||||||
{
|
{
|
||||||
QMacAutoReleasePool pool;
|
QMacAutoReleasePool pool; // FIXME Is this still needed for Creator? See 6a0bb4206a2928b83648
|
||||||
|
|
||||||
if (!m_enabled)
|
m_parentEnabled = enabled;
|
||||||
modal = true;
|
if (!m_enabled && enabled) // Some ancestor was enabled, but this menu is not
|
||||||
|
return;
|
||||||
|
|
||||||
foreach (QCocoaMenuItem *item, m_menuItems) {
|
foreach (QCocoaMenuItem *item, m_menuItems) {
|
||||||
if (item->menu()) { // recurse into submenus
|
if (QCocoaMenu *menu = item->menu())
|
||||||
item->menu()->syncModalState(modal);
|
menu->propagateEnabledState(enabled);
|
||||||
continue;
|
else
|
||||||
}
|
item->setParentEnabled(enabled);
|
||||||
|
|
||||||
item->syncModalState(modal);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -321,7 +321,7 @@ void QCocoaMenuBar::updateMenuBarImmediately()
|
|||||||
menu->setMenuParent(mb);
|
menu->setMenuParent(mb);
|
||||||
// force a sync?
|
// force a sync?
|
||||||
mb->syncMenu(menu);
|
mb->syncMenu(menu);
|
||||||
menu->syncModalState(disableForModal);
|
menu->propagateEnabledState(!disableForModal);
|
||||||
}
|
}
|
||||||
|
|
||||||
QCocoaMenuLoader *loader = getMenuLoader();
|
QCocoaMenuLoader *loader = getMenuLoader();
|
||||||
|
@ -97,10 +97,10 @@ public:
|
|||||||
NSMenuItem *sync();
|
NSMenuItem *sync();
|
||||||
|
|
||||||
void syncMerged();
|
void syncMerged();
|
||||||
void syncModalState(bool modal);
|
void setParentEnabled(bool enabled);
|
||||||
|
|
||||||
inline bool isMerged() const { return m_merged; }
|
inline bool isMerged() const { return m_merged; }
|
||||||
inline bool isEnabled() const { return m_enabled; }
|
inline bool isEnabled() const { return m_enabled && m_parentEnabled; }
|
||||||
inline bool isSeparator() const { return m_isSeparator; }
|
inline bool isSeparator() const { return m_isSeparator; }
|
||||||
|
|
||||||
QCocoaMenu *menu() const { return m_menu; }
|
QCocoaMenu *menu() const { return m_menu; }
|
||||||
@ -113,20 +113,21 @@ private:
|
|||||||
NSMenuItem *m_native;
|
NSMenuItem *m_native;
|
||||||
NSView *m_itemView;
|
NSView *m_itemView;
|
||||||
QString m_text;
|
QString m_text;
|
||||||
bool m_textSynced;
|
|
||||||
QIcon m_icon;
|
QIcon m_icon;
|
||||||
QPointer<QCocoaMenu> m_menu;
|
QPointer<QCocoaMenu> m_menu;
|
||||||
bool m_isVisible;
|
|
||||||
bool m_enabled;
|
|
||||||
bool m_isSeparator;
|
|
||||||
QFont m_font;
|
QFont m_font;
|
||||||
MenuRole m_role;
|
MenuRole m_role;
|
||||||
MenuRole m_detectedRole;
|
MenuRole m_detectedRole;
|
||||||
QKeySequence m_shortcut;
|
QKeySequence m_shortcut;
|
||||||
bool m_checked;
|
|
||||||
bool m_merged;
|
|
||||||
quintptr m_tag;
|
quintptr m_tag;
|
||||||
int m_iconSize;
|
int m_iconSize;
|
||||||
|
bool m_textSynced:1;
|
||||||
|
bool m_isVisible:1;
|
||||||
|
bool m_enabled:1;
|
||||||
|
bool m_parentEnabled:1;
|
||||||
|
bool m_isSeparator:1;
|
||||||
|
bool m_checked:1;
|
||||||
|
bool m_merged:1;
|
||||||
};
|
};
|
||||||
|
|
||||||
QT_END_NAMESPACE
|
QT_END_NAMESPACE
|
||||||
|
@ -88,16 +88,17 @@ NSUInteger keySequenceModifierMask(const QKeySequence &accel)
|
|||||||
QCocoaMenuItem::QCocoaMenuItem() :
|
QCocoaMenuItem::QCocoaMenuItem() :
|
||||||
m_native(NULL),
|
m_native(NULL),
|
||||||
m_itemView(nil),
|
m_itemView(nil),
|
||||||
m_textSynced(false),
|
|
||||||
m_menu(NULL),
|
m_menu(NULL),
|
||||||
|
m_role(NoRole),
|
||||||
|
m_tag(0),
|
||||||
|
m_iconSize(16),
|
||||||
|
m_textSynced(false),
|
||||||
m_isVisible(true),
|
m_isVisible(true),
|
||||||
m_enabled(true),
|
m_enabled(true),
|
||||||
|
m_parentEnabled(true),
|
||||||
m_isSeparator(false),
|
m_isSeparator(false),
|
||||||
m_role(NoRole),
|
|
||||||
m_checked(false),
|
m_checked(false),
|
||||||
m_merged(false),
|
m_merged(false)
|
||||||
m_tag(0),
|
|
||||||
m_iconSize(16)
|
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -133,15 +134,23 @@ void QCocoaMenuItem::setMenu(QPlatformMenu *menu)
|
|||||||
if (menu == m_menu)
|
if (menu == m_menu)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if (m_menu) {
|
if (m_menu && m_menu->menuParent() == this) {
|
||||||
if (m_menu->menuParent() == this)
|
m_menu->setMenuParent(0);
|
||||||
m_menu->setMenuParent(0);
|
// Free the menu from its parent's influence
|
||||||
|
m_menu->propagateEnabledState(true);
|
||||||
|
if (m_native && m_menu->attachedItem() == m_native)
|
||||||
|
m_menu->setAttachedItem(nil);
|
||||||
}
|
}
|
||||||
|
|
||||||
QMacAutoReleasePool pool;
|
QMacAutoReleasePool pool;
|
||||||
m_menu = static_cast<QCocoaMenu *>(menu);
|
m_menu = static_cast<QCocoaMenu *>(menu);
|
||||||
if (m_menu) {
|
if (m_menu) {
|
||||||
|
if (m_native) {
|
||||||
|
// Skip automatic menu item validation
|
||||||
|
m_native.action = nil;
|
||||||
|
}
|
||||||
m_menu->setMenuParent(this);
|
m_menu->setMenuParent(this);
|
||||||
|
m_menu->propagateEnabledState(isEnabled());
|
||||||
} else {
|
} else {
|
||||||
// we previously had a menu, but no longer
|
// we previously had a menu, but no longer
|
||||||
// clear out our item so the nexy sync() call builds a new one
|
// clear out our item so the nexy sync() call builds a new one
|
||||||
@ -184,7 +193,11 @@ void QCocoaMenuItem::setChecked(bool isChecked)
|
|||||||
|
|
||||||
void QCocoaMenuItem::setEnabled(bool enabled)
|
void QCocoaMenuItem::setEnabled(bool enabled)
|
||||||
{
|
{
|
||||||
m_enabled = enabled;
|
if (m_enabled != enabled) {
|
||||||
|
m_enabled = enabled;
|
||||||
|
if (m_menu)
|
||||||
|
m_menu->propagateEnabledState(isEnabled());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void QCocoaMenuItem::setNativeContents(WId item)
|
void QCocoaMenuItem::setNativeContents(WId item)
|
||||||
@ -392,12 +405,13 @@ void QCocoaMenuItem::syncMerged()
|
|||||||
[m_native setHidden: !m_isVisible];
|
[m_native setHidden: !m_isVisible];
|
||||||
}
|
}
|
||||||
|
|
||||||
void QCocoaMenuItem::syncModalState(bool modal)
|
void QCocoaMenuItem::setParentEnabled(bool enabled)
|
||||||
{
|
{
|
||||||
if (modal)
|
if (m_parentEnabled != enabled) {
|
||||||
[m_native setEnabled:NO];
|
m_parentEnabled = enabled;
|
||||||
else
|
if (m_menu)
|
||||||
[m_native setEnabled:YES];
|
m_menu->propagateEnabledState(isEnabled());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
QPlatformMenuItem::MenuRole QCocoaMenuItem::effectiveRole() const
|
QPlatformMenuItem::MenuRole QCocoaMenuItem::effectiveRole() const
|
||||||
|
Loading…
Reference in New Issue
Block a user