-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
window close
event gets ignored if nw2 is disabled / process
signals get ignored and NW.js crashes
#7981
Comments
I see crash too.
|
Yeah, we are still stuck on 0.66.1. I'm not sure it crashes, most likely it just exists the process without firing the event. It can be reproduced just by posting the following in console (shows alert in 0.66.1 and before, just exits in more recent version)
@rogerwang, could you please take a look at this one? I thought I saw this defect in the past, then it got fixed, but was re-introduced a few months back and it's been blocking upgrade to more recent versions for us as we rely on that feature. |
This is breaking bug for us. Close event is not triggered. |
NWjs 0.70 is broken too |
Yep... I've added v0.70.0 to my reproduction repo and updated the link in the OP. |
NW1 mode was deprecated three years ago: https://groups.google.com/g/nwjs-general/c/1YMHN3m1rtI/m/f3gF-Jl3AgAJ so bugs in nw1 would not be fixed. |
Second this issue. Just realized that my #8000 (that I now closed) is a duplicate of this. Given that nw2 is about 10x or more slower in processing scripted events than nw1, we really need nw1 to continue to be supported until nw2 reaches the same level of efficiency as nw1. See #7979 for more info. |
@rogerwang nw2 is 10x (or more slower) using scripting (see #7979 ) and should not be considered a replacement for nw1 yet. It literally renders my application unusable. Something that loads within a second on nw1 takes 10+ seconds to load in nw2 mode using the same version of nwjs. |
@rogerwang It appears #7943 also affects only nw2 but not nw1 which is another potential showstopper (cannot use CMD+(1-9) in nw2). I kindly ask that you please fix this issue as that is the only way I can go beyond 0.67.1. Thank you for your consideration. |
Duplicate of #7808 |
Issue Type
Issue
I'm reporting two issues that are somewhat related to each other when trying to intercept the termination of NW.js apps. I noticed the second issue while trying to debug the first one.
--disable-features=nw2
is set, attaching aclose
event listener to a NW.js window does not have any effect and the callback gets ignored.https://docs.nwjs.io/en/latest/References/Window/#event-close
process.removeAllListeners()
at the beginning of the application code (e.g. for being able to reset global application state in-between application window reloads) and then attaching an event listener on the process for aSIGTERM
signal for example, sending the signal to the running process will cause NW.js to crash if--disable-features=nw2
is set. If--disable-features=nw2
is not set, then the event listener gets ignored, which is bad.https://nodejs.org/api/process.html#signal-events
My application had to set
--disable-features=nw2
twice in the past because of various issues with the new nw2 implementation (1, 2), and so far I haven't reverted this (again). I will need to test the latest releases and see if those issues still occur, but the issues are fixed, I will upgrade to nw2, so the issue shouldn't affect my application. However, I felt like this issue is still worth reporting, as it's a regression that has been introduced in 0.68.The
process
event listener issue on the other hand looks like a NodeJS related issue (decided to test this after proof-reading my post). NW.js shouldn't crash though if--disable-features=nw2
is set.Reproduction
Please see my reproduction repo here with a simple NW.js app and a simple GitHub actions workflow:
https://github.com/bastimeyer/nwjs-window-close
The resulting log output with NW.js ranging from 0.64 to 0.70 while running several tests can be found here:
https://github.com/bastimeyer/nwjs-window-close/actions/runs/3384916071
The text was updated successfully, but these errors were encountered: