View Issue Details

IDProjectCategoryView StatusLast Update
0000857OpenMPTGeneralpublic2016-10-01 08:41
ReporterRapture Assigned ToSaga Musix  
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Product VersionOpenMPT 1.26.04.00 / libopenmpt 0.2-beta20 (upgrade first) 
Target VersionOpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first)Fixed in VersionOpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first) 
Summary0000857: More than one desired window get opened with certain key shortcuts
Description

For example, assigning "File/Export as lossless (Wave, FLAC)" to the key "U". When pressing U now 10 times, it opens the export dialogue 10 times on top of each other and you have to close 10 windows. This is not possible with clicking "File > Export as lossless (Wave, FLAC) with the mouse manually, as the file menu in the background is blocked when a window pops up (standard windows behaviour). With a shortcut to it, you can go around this "limitation" and open the dialogue as often as you want, which is undesirable.

  • You can only close the windows with leftclick on "X" or "cancel", or use Alt+F4 (don't hold down Alt+F4 or you may close the whole tracker application plus other running windows programs! ;)
  • Holding down "U" doesn't open many windows, but instead one, fortunately. Only pressing U repeatedly, opens too many window export dialogues.

Is that a thing that could be improved? I.e. no matter how many times you press the key "U" now, only and exactly one export dialogue will pop up and no more export windows can be created. Thanks!

TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Relationships

related to 0000713 new KeyboardHook can ultimately invoke a message loop which can cause re-entry to the hook function 

Activities

Rapture

Rapture

2016-08-08 17:55

reporter   ~0002575

Additional Info: You can create 14 windows max. ;) Why exactly 14, and if more/less/same on other dialogues than the export/wav/flac dialogue I don't know. ;)

manx

manx

2016-08-08 18:01

administrator   ~0002576

The root cause here is the same as issue 0000713, namely the global keyboard hook that works in parallel to the window hierarchy and practically ignores it.
The only proper solution here, is to remove the pointless keyboard hook and implement hotkey handling in all place and contextss where it is desired via the normal KEYUP/KEYDOWN/KEYPRESS window messages, or, if not handling delivered key messages, at least handle it in the message loop synchronously via PreTranslateMessage().

Rapture

Rapture

2016-08-08 18:11

reporter   ~0002577

Thanks for your reply. Can you please explain it again, so that a non-coder like me can understand it? :) What does "hotkey handling" mean, let alone pretranslatemessage()?

Why is that shortcut useless? No shortcut is useless if the user wants it. For one guy it's useless but for another one maybe not. ;) If the user wants the export dialogue with "U" (I don't want it myself, I just played around in OpenMPT!), then it's the users decision. Can't call that useless, really. Maybe that multiple-window-popup-thingy is typical Microsoft Windows behaviour, but I don't see that so often or at all in other programs.

manx

manx

2016-08-08 18:18

administrator   ~0002578

Thanks for your reply. Can you please explain it again, so that a non-coder like me can understand it? :) What does "hotkey handling" mean, let alone pretranslatemessage()?

My whole comment was meant as a development analysis and remainder for us developers about how to solve this properly, so that we do not forget it.
I should have mentioned that, sorry.

Why is that shortcut useless?

Same thing here. I meant that it is implemented the wrong way internally, not that keyboard shortcuts are anywhere near useless in general.

No shortcut is useless if the user wants it. For one guy it's useless but for another one maybe not. ;) If the user wants the export dialogue with "U" (I don't want it myself, I just played around in OpenMPT!), then it's the users decision. Can't call that useless, really. Maybe that multiple-window-popup-thingy is typical Microsoft Windows behaviour, but I don't see that so often or at all in other programs.

Your bug report is totally valid. I did not mean to question that ;-).

Sorry for the confusion caused.

Saga Musix

Saga Musix

2016-08-08 23:35

administrator   ~0002580

For the time being, we can disable the keyboard handler while the dialog is open, as it is already done with various other dialogs where it's not desired.

Rapture

Rapture

2016-08-09 01:09

reporter   ~0002582

@manx, hehe, no problem, and no harm done. :)
@Saga Musix: Sounds good then, for the time being.

Saga Musix

Saga Musix

2016-08-10 15:02

administrator   ~0002589

By the way @manx, I don't think that issue 0000713 is necessarily the culprit here. The fact that certain keyboard shortcuts work in all contexts is certainly intentional, but for some other shortcuts it is of course... less practical that this happens. For example, it can be handy to have play/stop/etc. available also while a child window is focused. Maybe we need to specially flag shortcuts which should only be usable when the main window is focussed instead of adding workarounds each and every dialog that has such a shortcut.

manx

manx

2016-08-10 15:42

administrator   ~0002591

Issue 0000713 is certainly related, as we are recursively entering the Keyboard Hook here. And that should never happen, ever, because it can totally confuse all state handling logic (this is not directly applicable to this issue, but think about the following situation: some complex logic like module cleanup spawns some confirmation dialog while in the process of modifying module data, and some other action gets launched via shortcut in the middle). If the keyboard hook was to send keypress messages to the GUI thread's message loop, one would handle the shortcuts where appropriate automatically, i.e. the main window handler would invoke the Export dialog, and if a modal dialog is already shown above the main window, the message would not even be processed by the main window, automatically resolving the superfluous window opening.
In any case, <strong>some</strong> flags for each and every shortcut determining where it can be available is probably due in any sensible solution, even if the async keyboard hook does not go away yet. In particular, anything that spawns dialogs probably needs some flag, as those are the most problematic here.

Saga Musix

Saga Musix

2016-08-17 23:12

administrator   ~0002612

Should be fixed in r6908, which should be on https://buildbot.openmpt.org/builds/ soon.

Issue History

Date Modified Username Field Change
2016-08-08 17:45 Rapture New Issue
2016-08-08 17:47 manx Relationship added related to 0000713
2016-08-08 17:55 Rapture Note Added: 0002575
2016-08-08 18:01 manx Status new => confirmed
2016-08-08 18:01 manx Note Added: 0002576
2016-08-08 18:11 Rapture Note Added: 0002577
2016-08-08 18:18 manx Note Added: 0002578
2016-08-08 23:35 Saga Musix Note Added: 0002580
2016-08-09 01:09 Rapture Note Added: 0002582
2016-08-10 15:02 Saga Musix Note Added: 0002589
2016-08-10 15:42 manx Note Added: 0002591
2016-08-17 23:12 Saga Musix Assigned To => Saga Musix
2016-08-17 23:12 Saga Musix Status confirmed => feedback
2016-08-17 23:12 Saga Musix Product Version => OpenMPT 1.26.04.00 / libopenmpt 0.2-beta20 (upgrade first)
2016-08-17 23:12 Saga Musix Target Version => OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first)
2016-08-17 23:12 Saga Musix Note Added: 0002612
2016-08-23 22:42 Saga Musix Status feedback => resolved
2016-08-23 22:42 Saga Musix Resolution open => fixed
2016-08-23 22:42 Saga Musix Fixed in Version => OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first)
2016-08-26 18:23 Saga Musix Fixed in Version OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) => OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first)
2016-08-26 18:23 Saga Musix Target Version OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) => OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first)
2016-08-26 18:24 Saga Musix Fixed in Version OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first) => OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first)
2016-08-26 18:24 Saga Musix Target Version OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first) => OpenMPT 1.26.05.00 / libopenmpt 0.2-beta20.1 (upgrade first)