Regression in JuceMainMenuHander on OSX


#1

Using latest develop branch my app now crashes as soon as I use the keyboard
which triggers menuNeedsUpdate then updateTopLevelMenu which then crashes due to some bogus stuff.

Still investigating.

It uses to work though :slight_smile:


#2

It calls getMenuForIndex with an index superior to the number of items declared in getMenuBarNames()
and I wasn’t returning an empty PopupMenu

Don’t know if it was supposed to do that.


#3

There have been some users reporting this but we can’t seem to reproduce this here. Any chance you could pm me a minimal example that reproduces the problem.


#4

Minimal exemple would be complicated to do.
Anyway it’s no biggie.
My function should have returned a PopupMenu in all cases, even empty.


#5

Have you tried using native mac menu bar ?


#6

Yep you guys have f… up when the menu bar is native mac.

Can you guys quickly fix this ? (I need to release my app today)

It’s very easily reproducible in the Juce demo once you switch to native mac menu in Tab & Widget example


#7

returning to 30 dec 2016 version of juce_mac_MainMenu.mm fixes the issue


#8

Should be fixed on develop now


#9

Looks like there is still something weird going on. (latest develop trunk)
The first time I select the menu first items of the first column the topLevelMenuIndex of menuItemSelected callback is offseted by 1 then the second time it is correct (get 1 as topLevelMenuIndex then 0)
If I choose first the second column and then choose it again, I always get topLevelMenuIndex to 2 which is wrong as well as it should be 1 (0 based)


#10

@Jules Can you guys can have a look please ?


#11

https://pastebin.com/WDjuccz7
If you replace hello World Main.cpp code with this one.
You will be able to reproduce the changes between mac native menu bar and default juce one.

topLevelMenuIndex will not have the same value in both case.
This is annoying if you have duplicated menuItemId between columns.


#12

Thanks for reporting. I think this is fixed now on develop with commit 51311ce. Can you confirm that this works for you?


#13

Looks good now.

Thanks !