"bogus shift keys in Mac menus" bug - found!

I’ve reported a couple of times before that there were bogus shift keys inserted into the menu descriptions on the Mac platform.

I’ve finally tracked down the problem to juce_KeyPress.cpp.

#0 0x0040c475 in juce::KeyPress::createFromDescription at juce_KeyPress.cpp:226
#1 0x0040f5ed in juce::KeyPressMappingSet::restoreFromXml at juce_KeyPressMappingSet.cpp:254

Here’s what happens!

[] The KeypressMappingState is restored from XML.[/]
[] A single key command line “command + z” is read.[/]
[] This goes to createFromDescription.[/]
[] This successfully reads off the "command + " segment.[/]
[] But then it incorrectly calls CharacterFunctions::toUpperCase (desc.getLastCharacter()); resulting in a character code of 90 (‘Z’) instead of 122 (‘z’).[/]
[] The Apple menu renderer code sees ‘Z’ and renders that as “[shift symbol] Z”.[/]
[] Funky menus![/][/list]
I don’t know whether this code is wrong for Windows and Linux as well.

For the moment, I changed toUpperCase to toLowerCase, but perhaps even it should just be left alone?

Would a good fix be to just change the code in juce_mac_MainMenu.mm:

[item setKeyEquivalent: juceStringToNS (String::charToString (key).toLowerCase())];

Removing my hack and replacing it with your fix seems to have the identical effect… I’m not so familiar with the code to know which is better!

Ok, thanks! I think that since it’s just the mac menu that’s exhibiting a problem, it’s best to fix just that section; if I changed KeyPress to work differently, there might be code elsewhere that could be affected by that.

Bummer. I updated to the tip, which has the change to juce_mac_MainMenu.mm, and I’m now getting those spurious Shift markers again.

Putting in my original fix still works. I went through the chain of debugging quickly again, and it still seems like with the current tip, there is no way for a Mac user to not get spurious shifts in menu items that are created by keystroke serialization… i.e., any menu item that’s created from the KeyPressMappingSet.

I must have botched the test of your change somehow! I was sure I turned off mine, but I suppose I didn’t. I have a static variable in there now…

Sorry. :-/

But that doesn’t make any sense…

If it’s the capitalisation of the ‘N’ that’s making the OSX stuff add the ‘shift’, then my fix should work.

…but if that doesn’t work, it’d imply that the Keypress object must already contain a ‘shift’ modifier bit. But I can’t see any code path where an input of “command + N” could produce a Keypress with a shift modifier…


Sorry for the delay, I’d foolishly turned “Notify Me” off.

OK, I’m going to go through this step at a time and report exactly what happens.

It should be easy to reproduce in any Mac application that stores and recalls keyboard assignments as XML - when I first ran this issue in June, I was able to add an “edit key assignment” function to either the demo or the Jucer (can’t remember which) with little code and repro it there.

Or I might be able to put out a small command line program that demonstrates it! Either way, stay tuned…

Aha! I believe now that nothing is wrong with our understanding of the issue or your change.

The issue was that the program is using a keyboard assignment data file written by an earlier version of Juce. Once I replaced that file with an all lower-case one it now all exhibits the desired, fixed behavior.

There’s still a slight weirdness where the ! key is displayed as “shift + !” - we discussed this before. In the idea world, you might want either “!” or “shift + 1” as the representation of !.. but this is small beans…

Ah! Glad you found that, it was very puzzling.

The “shift + !” problem is particularly tricky, because there’s no sensible way to actually find out that “!” is the same as “shift + 1”. And on some keyboards, of course, it won’t be. So for some people, “shift + !” might make perfect sense. And also, if the user assigns a key-shortcut by pressing shift + 1, you’ve no way of knowing whether they’re thinking of it as “shift + 1”, or whether their intention was really for it to be the “!” key…

I agree of course with your comments on “shift + !” - you’d clearly need the programmer to be able to specify which version they were looking for (and now you brought up the different keyboard idea, there must be some keyboards where ! is not a shift character at all…!) BUT it’s not clear how “I” get to have a say on how to display a keystroke when “you” have to serialize the keystroke name (well… I could give you a table of names to look up, it wouldn’t really be that hard, but not a very important feature for the amount of work).

Onwards and upwards!