Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#10342 closed defect (fixed)

Menu accelerators aren't removed

Reported by: Cyball Owned by:
Priority: normal Milestone:
Component: old wxOSX/Carbon port Version: 2.8.9
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

Attempting to remove a menu item's shortcut by calling SetText(), doesn't actually remove the shortcut.

For instance, setting the shortcut works just fine:

item->SetText(_("Testme\tSHIFT+T"));

But, attempting to unset it doesn't:

item->SetText(_("Testme"));

The reason is that wxMenuItem::UpdateItemText() calls wxAcceleratorEntry::Create() to parse the shortcut, but since there isn't one wxAcceleratorEntry::Create() returns NULL. That's okay and UMASetMenuItemShortcut() expects it. However, UMASetMenuItemShortcut() immediately returns without actually removing any pre-existing shortcut.

I've attached a possible fix. It works here, but I was unable to locate any reference for the added function calls, so it may not be correct.

Leland

Attachments (1)

remove_shortcut.diff download (568 bytes) - added by Cyball 12 years ago.

Download all attachments as: .zip

Change History (8)

Changed 12 years ago by Cyball

comment:1 Changed 12 years ago by csomor

the fix is absolutely to the point and correct, I'll commit

thanks a lot

stefan

comment:2 Changed 12 years ago by SC

  • Resolution set to fixed
  • Status changed from new to closed

(In [57897]) clearing shortcuts, patch from Leland, fixes #10342

comment:3 Changed 12 years ago by Cyball

  • Resolution fixed deleted
  • Status changed from closed to reopened

Stefan,

Just been doing a little testing on Tiger and the SetItemCmd() is causing some display issues.

If the SetItemCmd() is done, then clicking one menu (to open it) and then moving the pointer from that menu to the next causes the glyphs and modifiers to disappear on all but the application menu. If the SetItemCmd() call is removed, then everything appears to work properly.

This does not happen on Leopard.

I can provide a video if that'll help.

Also, I should explain what we're doing.

We define our menus without accelerators so that we can capture all of the key events as we provide our own "accelerator-like" processing. If we defined them with accelerators, we would not be able to handle the key events ourselves. (Some key events do different things based on the active display object.)

But, we want the user to see the accelerators assigned to menu items, so when we receive a wxEVT_MENU_OPEN event, we do a SetText() (with accelerator) on each item that lives on the menu that's opening. When the event handler finishes, the menu is displayed with the proper glyphs and modifiers.

Then, when the wxEVT_MENU_CLOSE event comes round, we do the SetText()s again, but without the accelerators so we again receive all of the key events.

I can provide sample code if you need it.

Leland

(I didn't know if I should "reopen" it or not.)

comment:4 Changed 12 years ago by Cyball

  • Resolution set to fixed
  • Status changed from reopened to closed

I have to apologize Stefan. I was wrong. The patch is working just fine. I needed to handle the Mac differently during the menu open/close events since it doesn't behave quite like GTK and MSW. Once I realized that, the menus are updating just fine.

So, I'm going to close this ticket (if it will let me) and please forgive my jumping to conclusions.

Thanks,

Leland

comment:5 Changed 12 years ago by csomor

Hi Leland

thanks for pursuing this further, could you please summarize what behaves differently and which workaround worked, so that it may be found again, if someone stumbles upon the same problems later.

Thanks,

Stefan

comment:6 Changed 12 years ago by Cyball

Sure thing...

I thought about it some more after my last comment and know exactly why it acts differently in our case.

As mentioned, during a wxEVT_MENU_OPEN event we add the accelerator to the menu items that are about to be displayed and we remove them during wxEVT_MENU_CLOSE event.

Well, we actually had a bug in our routines that do this but it didn't show up until the above patch was applied. The reason it didn't show up was because during the close event, the menu accelerators were never being removed. We didn't realize this until last week.

During the Open event, we kept a pointer to that menu to use during the Close event. This is necessary because on MSW and GTK, the wxMenuEvent::GetMenu() method can return NULL, so there's no way of knowing what menu was closing. Also, a Close event isn't generated after a menu has been opened when the mouse or keyboard causes another menu to open on those platforms.

Our bug was that we assumed Mac worked the same way. But, it's actually the only one of the three that actually works correctly.

For every menu that opens, an open and close event is generated, even if moving from menu to menu. In the latter case, the events come in as open menu1/open menu2/close menu2/close menu1. But, that's okay since GetMenu() returns the correct menu making it unnecessary to retain a pointer to the previously opened menu.

The problem with referencing the previously opened menu during the close event is that it's not the object of the event and the modifier and glyph icons start to disappear if you do. I read somewhere that during these events, you shouldn't update any other menu...only the one the event is for. Unfortunately, I can no longer find that reference.

So, my solution is to not try and "over manage" the menu updating on the Mac and simply use the menu object that GetMenu() returns. Works perfectly now...

The change in behavior, as far as wxWidgets goes, is that it's actually working as expected now. And the lesson learned was not to modify any menus other than the one that generated the wxMenuEvent on the Mac.

Leland

comment:7 Changed 12 years ago by csomor

perfect, thanks for the write-up

Stefan

Note: See TracTickets for help on using tickets.