Skip to content
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 doesn't close with close event handler and window.close() #7808

Open
sysrage opened this issue Nov 18, 2021 · 6 comments
Open

Window doesn't close with close event handler and window.close() #7808

sysrage opened this issue Nov 18, 2021 · 6 comments
Assignees
Labels
has-min-repro Has a minimum reproduction regression triaged

Comments

@sysrage
Copy link

sysrage commented Nov 18, 2021

NWJS Version : 0.58.0
Operating System : Windows 10

Expected behavior

When a window uses window.close(), handling of the close event should behave the same as if nw.Window.get().close() was called or the user manually closed the window.

Actual behavior

If window.close() is used to close a window, an event handler cannot be attached to the close event. If an event is attached, the window does not actually close when win.close(true) is called.

How to reproduce

  1. Open NW.js SDK and open the DevTools console.
  2. Execute:
nw.Window.get().on('close', function() {
  this.close(true);
});
  1. Execute:
window.close();
  1. Note that the window does not close. However, the user can no longer interact with the window.
@TheJaredWilcurt
Copy link
Member

TheJaredWilcurt commented Nov 18, 2021

Tested locally and could reproduce in NW.js 0.58.0-SDK on Win 10 64:

  • window.close() - Closes dev tools, ends UI interactivity, but the window and UI is still visible.
  • nw.Window.get().close() - closes the window and dev tools. All processes end.
  • nw.App.quit() - Closes the window and dev tools. All processes end.
  • process.exit(0) - Closes dev tools. UI removed but window stays open.
  • process.exit(1) - Closes dev tools. UI replaced with "Aw, Snap! RESULT_CODE_KILLED" Chromium error page.

Min Repro:

@TheJaredWilcurt
Copy link
Member

TheJaredWilcurt commented Nov 18, 2021

EDIT: Updated this, I put "0.50.0" as verified good when it should be verified bad.

Verified bug is not present in:

  • 0.40.0

Verified bug is present in:

  • 0.45.0
  • 0.50.0
  • 0.50.1
  • 0.50.2
  • 0.50.3
  • 0.51.0
  • 0.52.0
  • 0.53.0
  • 0.55.0
  • 0.58.0

Diff:

@rogerwang

@ev8dev
Copy link

ev8dev commented Nov 19, 2021

Yesterday I was struggling with process.exit() as well after updating my NW.js version. I need to quit my application with a 0 or 1 as exit code to capture this with a second application while closing all NW.js related processes.

@rogerwang
Copy link
Member

Thanks for the sample and testing. but the bug is in 0.50.0 as well

@TheJaredWilcurt
Copy link
Member

TheJaredWilcurt commented Nov 29, 2021

nw7808.zip

Version win.close() nw.App.quit() window.close() process.exit(0) process.exit(1)
0.42.0 ✔️ ✔️ ✔️ ✔️ ✔️
0.42.1 ✔️ ✔️ ✔️ ✔️ ✔️
0.42.2 ✔️ ✔️ ✔️ ✔️ ✔️
0.42.3 ✔️ ✔️ ✔️ ✔️ ✔️
0.42.4 ✔️ ✔️ ✔️
0.45.0 ✔️ ✔️ ✔️
0.46.0 ✔️ ✔️ ✔️
0.46.1 ✔️ ✔️ ✔️
0.46.2 ✔️ ✔️ ✔️
0.46.3 ✔️ ✔️
0.58.0 ✔️ ✔️

@rogerwang Updated table with tested versions and relevant diffs

@ssnangua
Copy link

An interim solution, Works for me :)

chrome.tabs.onRemoved.addListener(() => {
    nw.Window.getAll((list) => {
        list.forEach((win) => {
            if (win.cWindow.tabs.length === 0) {
                win.close(true);
            }
        });
    });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-min-repro Has a minimum reproduction regression triaged
Projects
None yet
Development

No branches or pull requests

5 participants