Tweak wxExec to send WM_QUIT and change test app to mspaint (PR #23408)
MetalPlateMage
Change wxKill to PostMessage(WM_QUIT) to each thread of a process, the same as Windows does when it tries to kill a process. If we want to do more of what windows does we'd do a WM_CLOSE and WM_ENDSESSION to each topwindow of a process, but it might be just excessive...
Change the test application to use for tests from Notepad to MSPaint, which at least as the moment doesn't do a process pool or anything so it quits cleanly and I'm not left with Notepads open every time I run the tests.
This technically "resolves" #23365 (#23365).
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/23408
Commit Summary
- b2e61c1 Sync with my local copy, except for Normalization (my local copy is in almost pure C, so it doesn't port over cleanly):
- c3cca75 Fix github issue #3350 - it was already fixed, but the assert was bad for old code in Resolve(), so I changed it for compatibility purposes
- 45e0dc8 Change wxKill to PostMessage(WM_QUIT) to each thread of a process, the same as Windows does when it tries to kill a process.
- 097cad9 Merge branch 'master' of https://github.com/wxWidgets/wxWidgets into wxExectweaks
- c0b45a8 Merge branch 'master' of https://github.com/wxWidgets/wxWidgets
- a8a0c17 Merge branch 'wxExectweaks' of https://github.com/MetalPlateMage/wxURI-updates
File Changes
(2 files)
- M src/msw/utils.cpp (79)
- M tests/exec/exec.cpp (2)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/23408.patch
- https://github.com/wxWidgets/wxWidgets/pull/23408.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks!
Just to confirm: does the current code fail to kill mspaint too? I.e. do I understand correctly that just changing the program used in the test is not enough, and that we also need to change wxKill() implementation to make it work with mspaint, but that even this change to wxKill() is still not enough to make it work with notepad because of its child processes?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
MetalPlateMage
-
Windows 11 notepad uses process pooling. Currently the only way I can think of without using WPF/.NET is to hook CreateProcess() when you spawn the process - I'm sure there are other ways, but so far all of them are beyond what wxKill() is stated to do.
-
Just switching to mspaint with the current code passes the tests - just tested now (didn't originally since I implemented this first in my quest to kill notepad).
-
This way works even if the process doesn't have a top level window created like the parent notepad process... usually. It doesn't kill the ones it spawns that show the GUI though as I mentioned. I can understand not wanting to change stable code.
Hope those answer your questions Vadim.
P.S. Also sorry if there were any weird pull requests before this; I have very little usage with git - I've used svn for over 20 years.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks for the answers!
I indeed prefer not to change the existing code if it has no immediate benefits. Perhaps we can replace it with something more reliable, that works with notepad too, later, but for now I'll apply just the change to the test to make it pass under Windows 11 but not the rest of them.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #23408.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.