Fixed #1091 by reverting 7f0f89

[JonnyPtn]

This reverts commit 7f0f89b.

Assuming this commit was meant to address #527, I can't reproduce any of the original symptoms on 10.12.2 with it reverted

[mantognini]

Thanks a lot Jonny for your analysis of the problem for #1091! I unfortunately don't have time to test it now, but hopefully someone else in the community can.

If this indeed fixes the issue and doesn't reintroduce the bug mentioned in #527 I'm fine with it -- getting ride of a ugly workaround is always good. It would be interesting to test the behaviour on an older versions of the operating system though.

Alternatively, maybe the issue is with the implementation of validateMenuItem:. Maybe it should accept other action as well...

[JonnyPtn]

From the docs: "Typically you should use validateUserInterfaceItem: instead of validateMenuItem: because the former will also work for toolbar items which have the same target and action."

I guess at this point it needs testing on other OS versions, as I've only tested on 10.12. If this causes regression on some versions then I'll test using validateUserInterfaceItem instead

https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/MenuList/Articles/EnablingMenuItems.html

[eXpl0it3r]

@mantognini / @JonnyPtn Do you think we should go ahead with this and if there are reports that it broke something else, we can revisit it again?

I rather have something fixed in the OS X's current version, than stalling due to a potential issue in some older OS X version.

[JonnyPtn]

It certainly seems to address the issue here, but it might be worth confirming it works on another machine before going ahead, just in case

For anyone else who can test, essentially just need to verify that you can use the window menu as well as the keyboard shortcut (cmd+Q) to quit the program

[mantognini]

Please ping me by the end of next week if I forgot -- I should be able to give this a shot.

[mantognini]

So I've tested this, but unfortunately it reintroduces #527. :-/

You said you couldn't reproduce it? I don't have the latest OS version (will upgrade to macOS 10.12 soon probably). Maybe, although unlikely, Apple did change something with its latest one.

Here's how I've tested your PR:

1. removed any installation of SFML on my machine
2. git checkout / merge your branch
3. compiled and installed the debug-dylib version (*) of SFML and its dependencies.
4. download SFML-Test-Events and open its Xcode project
5. run it and observe the following:
   • cmd+w or cmd+q emit a close even in window mode ("Close" should be printed in the log/window).
   • after pressing f to enter fullscreen mode, cmd+w or cmd+q do not emit events and the later result in a system alert.
   • esc closes the programs.

Note: lines 428 to 485 can comment out to get a less verbose log.

(*) if regular dylibs or frameworks are used instead, the Xcode project of SFML-Test-Events needs to be updated to reflect this change; see instructions here.

---

The good news is that I've managed to fix validateMenuItem:. (The doc you quoted above seems to be for another class/protocol, thankfully.) Could you give a shot and try out branch bugfix/osx_window_menu? See PR #1193.

Pinging @thedeadc0der as well since you reported #1091.

[mantognini]

Given the positive feedback on #1193 I'm closing this PR. Although your work was not merged I highly appreciate the fact you've found which commit was at fault, so thanks again! 👍

[JonnyPtn]

Very confusing, I can't for the life of me work out why it doesn't regress on my machine...

Glad it's fixed anyway!