Alt-F4 on other application shuts down edex-ui

sadata7 commented 3 years ago

Technical information

Using version:

[ ] master (running from GitHub-published source code, currently v2.2.7-pre)
[x] latest (latest release, currently v2.2.6)
[ ] vX.X.X (specify other version)

Running on:

[x] Linux
[ ] Windows
[ ] macOS

How comfortable you are with your system and/or IT in general:

[ ] I'm kind of lost, honestly
[ ] I know what's up, I could help you run some commands or checks
[x] My machine is fully under my control, tell me what you need
[ ] I attended Defcon last year

Problem

Pressing Alt-F4 to shutdown another application (e.g. text editor, calculator, etc.) on the same desktop/workspace as edex-ui also shuts down edex-ui. It doesn't matter if the other app is started from the edex-ui terminal or not.

wrac4242 commented 3 years ago

which desktop manager/distro are you using?

sadata7 commented 3 years ago

which desktop manager/distro are you using?

Linux Mint 20.1 Cinnamon

GitSquared commented 3 years ago

Do you happen to use multiple monitors?

sadata7 commented 3 years ago

No, just a single monitor on this machine. Note that it only happens when the app is on the same desktop.

I think this is a debounce issue (i.e. edex-ui is processing queued up keystrokes) because, once or twice, I was able to hit the Ctrl-F4 key combination very briefly/quickly and it only affected the intended app (e.g. editor, calculator, etc.). But the majority of the time it just passes right through to edex-ui, shutting it down.

GitSquared commented 3 years ago

Any performance issues with the app or is it running smoothly?

sadata7 commented 3 years ago

Seems very smooth.

GitSquared commented 3 years ago

Have you switched the forceFullscreen/allowWindowed settings in edex or are you running it fullscreen, using your WM to move windows on top?

https://github.com/GitSquared/edex-ui/blob/ec8e383b892a893e3d47ba7e39001fd3021c612b/src/_boot.js#L183

sadata7 commented 3 years ago

Same behavior whether in windowed or full-screen mode. However, if I open more than one app such that there is another app between (i.e. in the window list) edex-ui and the app intended to receive Alt-F4, then there's no problem (nor does the intermediate app close unexpectedly if I hit Alt-F4 on the other app). So, for example, if I launch edex-ui (from 2.2.6 AppImage file) and then launch gnome-calculator (either from edex-ui terminal or Ulauncher), closing gnome-calculator with Alt-F4 will immediately shutdown edex-ui. However, if I first launch, say, xed editor or gnome-calculator, and then a second instance of either app, then the intermediate app prevents Alt-F4 from being heard by edex-ui. Only the app intended to receive Alt-F4 shuts down and, at this point, focus is received on the intermediate app. However, hitting Alt-F4 on the remaining app will shutdown that app as well as edex-ui. So problem occurs on apps one level higher than edex-ui in the window list. Also, the launch order doesn't seem to matter. If I launch gnome-calculator or xed editor first, and then edex-ui, bringing the other app to the foreground (e.g. via Alt-Tab) and hitting Alt-F4 still shuts down edex-ui.

sadata7 commented 3 years ago

Looks like fix 265 was specific to Windows. I guess you could update as below so that the fix only applies to Windows. I tested the code below and it resolves the issue on Linux (and Alt-F4 sent directly to edex-ui still shuts it down, as expected). Don't have Windows to test on, but I assume it will work.

// Fix #265
window.addEventListener("keyup", e => {
    if (require("os").platform() === "win32" && e.key === "F4" && e.altKey === true) {
        electron.remote.app.quit();
    }
});

indramardiana commented 3 years ago

hello bro ... I'm a beginner ... I was betrayed by my partner ... I want to take a conversation on my partner's cellphone from a distance, can you help tapping the WhatsApp bro

Pada tanggal Rab, 10 Mar 2021 10:02, sadata7 notifications@github.com menulis:

I commented out lines 1105 to 1109 and it resolved the issue (and Alt-F4 sent directly to edex-ui still shuts it down on Linux, as expected). Looks like fix 265 was specific to Windows. I guess you could update as below so that fix only applies to Windows:

// Fix #265 window.addEventListener("keyup", e => { if (require("os").platform() === "win32" && e.key === "F4" && e.altKey === true) { electron.remote.app.quit(); } });

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GitSquared/edex-ui/issues/1063#issuecomment-794990789, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATFFXDES3OZYAR3BUAEITEDTC4DONANCNFSM4YXOL37Q .

GitSquared commented 3 years ago

So, for example, if I launch edex-ui (from 2.2.6 AppImage file) and then launch gnome-calculator (either from edex-ui terminal or Ulauncher), closing gnome-calculator with Alt-F4 will immediately shutdown edex-ui.

Very weird behaviour, could you make sure that it doesn't depend on whether you launch new windows from the edex terminal? I'm thinking they may spawn as child processes and cause edex to still listen to keyboard events in the background.

I guess you could update as below so that the fix only applies to Windows. I tested the code below and it resolves the issue on Linux

The fix is only applying on Windows (require("os").platform() === "win32"). Did you mean to say that you removed that check and it worked on your end?

sadata7 commented 3 years ago

As mentioned, it doesn't matter if the apps are launched from the edex-ui terminal or using Ulauncher or Alt-F2 launcher. Also, as mentioned, they can be launched before edex-ui is even launched.

As for the fix, I added the require("os").platform() === "win32". That's why I said you "could update it as below." The OS check was not part of your original fix (see your post above where you quoted the current code).

GitSquared commented 3 years ago

Right, sorry for the misunderstanding - and thanks for digging in and finding a fix. I'll update it, unless you want to send a PR.

sadata7 commented 3 years ago

Right, sorry for the misunderstanding - and thanks for digging in and finding a fix. I'll update it, unless you want to send a PR.

Not prob. I should have said the "issue" was Windows specific, not the fix. When I looked up #265, it mentioned that it occurred on Windows and I believe you or someone else couldn't reproduce on Linux. I haven't done a PR before, so I can give it a try for this as well as the uptime format later today.

GitSquared commented 3 years ago

Great! Just remember to make two different PRs. If this is your first time, the GitHub guide is a good place to start.

sadata7 commented 3 years ago

Great! Just remember to make two different PRs. If this is your first time, the GitHub guide is a good place to start.

I created the first PR and submitted. I then recloned from my fork and made the second change, committed and pushed. However, it seems to have automatically pulled in the second change as part of the existing pull request. Not sure how I can separate them. Please advise.

GitSquared commented 3 years ago

PRs mean "please merge this branch with this branch" (in the one you opened, #1072, the request is "merge my fork's master branch with your repo's master branch"). So if you add commits to the branch they will be added to the changes proposed in the PR.

The solution is to make different branches on your end to separate the new features/fix, for example fix-265 and feat-better-uptime. Then you can send two different PRs and I can review them independently.

sadata7 commented 3 years ago

Should be in good order now. Thanks.

GitSquared commented 3 years ago

@sadata7 Thanks for taking the time to do this. I've merged both PRs, you're now officially a contributor! :tada:

GitSquared / edex-ui

Alt-F4 on other application shuts down edex-ui #1063

Technical information

Problem