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

Quit on macOS from top or dock icon menus does not have "quit" value in "close" event #7365

Open
arudnev opened this issue Feb 7, 2020 · 9 comments

Comments

@arudnev
Copy link

arudnev commented Feb 7, 2020

This is related to #7242 and #7355, but I'm specifically concerned about quit parameter of close event

NWJS Version : 0.44.0
Operating System : macOS Catalina 10.15.3

Expected behavior

Clicking Quit on dock icon menu or Quit nwjs from top menu should trigger close event with quit='quit'

Actual behavior

In nw1 mode close event is triggered with quit='quit' on Quit nwjs from top menu and not triggered at all for Quit from dock icon menu (the later is a bit unexpected, but ok)

In nw2 mode close event is triggered with quit=undefined on Quit nwjs from top menu or Quit from dock icon menu.
It's triggered as expected with quit='quit' on control + Q though.

How to reproduce

Running the following in dev tools console and clicking Quit on dock icon menu or Quit nwjs from top menu should do it:

nw.Window.get().on('close', (q) => { alert(`quit: ${q}`); q && nw.App.quit(); })
@rogerwang
Copy link
Member

rogerwang commented Feb 10, 2020

The current behavior aligns with the document and is working as expected -- it is set to quit only when Cmd-Q is pressed.

@arudnev
Copy link
Author

arudnev commented Feb 10, 2020

@rogerwang, thanks for quick reply.

What we are really looking for is to be able to implement behavior that is somewhat standard for messaging app.
Take Skype for example. If you select Quit from dock icon menu or Quit Skype from top menu it would exit the app, but if you click close button on the window frame it would just close the window, leaving the app running.
It's possible in nw1 mode, but no longer possible in nw2 mode.

Also, in regards to the top menu, it says Quit nwjs and shows Cmd+Q as shortcut, so it's expected that behavior for Cmd+Q would be similar to what you would get from clicking on menu button, but again, in nw2 mode it's no longer possible to distinguish click on the menu item and trigger actual quit of the app from click on close button on windows frame, as it was possible in nw1 mode.

Screenshot 2020-02-10 10 26 34

If behavior for click on Quit nwjs in top menu and click on Quit in dock icon menu changed in nw2 mode, is it possible to maybe add different values for the quit parameter of close event (i.e. "quit", "menu", "dock") if passing "quit" in all those cases does not make sense for some reason, so that we could somehow distinguish intent to actually quit the app from intent to close the window?

@rogerwang
Copy link
Member

This is fixed in git and will be available in the next nightly build.

Any further issues please let me know.

@arudnev
Copy link
Author

arudnev commented Feb 11, 2020

Hi @rogerwang, it seems that the fix made it to 0.44.1 according to readme, thank you.

We can see quit now when Quit nwjs menu item is clicked in the top menu, but it seems that when we do Quit from dock icon menu we still don't get quit and app is not exiting (as it would do in nw1 mode, without even firing close event).

So, we still can not distinguish between intent to close the window (close button on window frame) and quit the app (i.e. clicking Quit menu item on dock icon).

By the way, with --disable-features=nw2 we are getting the following in the console, I don't think it's expected (it's on macOS Catalina 10.15.3), even though does not seem to break anything.

~/Downloads/nwjs-sdk-v0.44.1-osx-x64/nwjs.app/Contents/MacOS/nwjs --disable-features=nw2

[22736:775:0211/120933.368737:FATAL:keep_alive_registry.cc(105)] Check failed: !is_shutting_down_.
0   nwjs Framework                      0x000000010d9bc969 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 15158777
1   nwjs Framework                      0x000000010d8eb143 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 14300627
2   nwjs Framework                      0x000000010d8ff3bb v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 14383179
3   nwjs Framework                      0x000000010bb976bd ChromeMain + 30612237
4   nwjs Framework                      0x000000010d4b7f14 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 9896868
5   nwjs Framework                      0x000000010d3e5d36 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 9036230
6   AppKit                              0x00007fff295e7980 -[NSApplication _terminateFromSender:askIfShouldTerminate:saveWindows:] + 126
7   AppKit                              0x00007fff295e788b __52-[NSApplication(NSAppleEventHandling) _handleAEQuit]_block_invoke + 46
8   AppKit                              0x00007fff2962f91c ___NSMainRunLoopPerformBlockInModes_block_invoke + 25
9   CoreFoundation                      0x00007fff2bfcc7ab __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
10  CoreFoundation                      0x00007fff2bfcc6ed __CFRunLoopDoBlocks + 379
11  CoreFoundation                      0x00007fff2bfcb731 __CFRunLoopRun + 1257
12  CoreFoundation                      0x00007fff2bfcabd3 CFRunLoopRunSpecific + 499
13  HIToolbox                           0x00007fff2ab2065d RunCurrentEventLoopInMode + 292
14  HIToolbox                           0x00007fff2ab202a9 ReceiveNextEventCommon + 356
15  HIToolbox                           0x00007fff2ab20127 _BlockUntilNextEventMatchingListInModeWithFilter + 64
16  AppKit                              0x00007fff29190ba4 _DPSNextEvent + 990
17  AppKit                              0x00007fff2918f380 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
18  nwjs Framework                      0x000000010d42ba90 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 9322272
19  nwjs Framework                      0x000000010d9cbb0a v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 15220634
20  nwjs Framework                      0x000000010d42b9c9 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 9322073
21  AppKit                              0x00007fff2918109e -[NSApplication run] + 658
22  nwjs Framework                      0x000000010d9d3e8c v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 15254300
23  nwjs Framework                      0x000000010d9d2432 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 15247554
24  nwjs Framework                      0x000000010d966c98 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 14807336
25  nwjs Framework                      0x000000010d932689 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 14592793
26  nwjs Framework                      0x000000010d433cb3 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 9355587
27  nwjs Framework                      0x000000010b13370b ChromeMain + 19716955
28  nwjs Framework                      0x000000010b135612 ChromeMain + 19724898
29  nwjs Framework                      0x000000010b130695 ChromeMain + 19704549
30  nwjs Framework                      0x000000010d2026dd v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 7056237
31  nwjs Framework                      0x000000010d2023e9 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 7055481
32  nwjs Framework                      0x00000001104a720c v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 60159644
33  nwjs Framework                      0x000000010d2015e8 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) + 7051896
34  nwjs Framework                      0x0000000109e65cc7 ChromeMain + 279
35  nwjs                                0x00000001085bd384 main + 372
36  libdyld.dylib                       0x00007fff636707fd start + 1
37  ???                                 0x0000000000000002 0x0 + 2

[0211/120933.381961:WARNING:process_memory_mac.cc(93)] mach_vm_read(0x7ffee7643000, 0x2000): (os/kern) invalid address (1)
[0211/120933.496789:WARNING:system_snapshot_mac.cc(42)] sysctlbyname kern.nx: No such file or directory (2)
[0211/120933.608771:WARNING:crash_report_exception_handler.cc(239)] UniversalExceptionRaise: (os/kern) failure (5)

[1]    22736 trace trap  ~/Downloads/nwjs-sdk-v0.44.1-osx-x64/nwjs.app/Contents/MacOS/nwjs

This error is not logged if you open Devtools prior to clicking Quit on dock icon menu.

@aliblackwell
Copy link

Hey, yes I have just come to the exact same conclusion. This fix has worked beautifully for quitting from the top menu, but quitting from the dock icon menu still just hides the window rather than properly quitting the application.

I have verified this using 0.44.1 using npm start and also doing a full build for mac and running it.

@aliblackwell
Copy link

I've updated our nw2_quit_bug mini project to show the behaviour. If you run the app and then use the new "Settings" option in the tray menu to open a window, you will not be able to quit the app using the dock icon once the window has been opened.

Note how in the close event I am hiding the window unless the function has been passed the "quit" value. It seems this value still isn't being passed from dock icon quit.

menuItemOpenSettings.on("click", () => {
  nw.Window.open("./index.html", windowSettings, function(win) {
    win.on("close", function(c) {
      this.hide()
      if (c === "quit") {
        nw.App.quit()
      }
    })
  })
})

Should we create a new issue or re-open this one @rogerwang? Thanks so much for your work and for the new release earlier on! Hopefully this is an easy fix.

@akshay-joshi
Copy link

akshay-joshi commented Feb 22, 2021

Hi

Today I install the latest version 0.51.2 and the issue is still reproducible. When I click on the Quit option from Dock 'close' event is not fired. I need to do some cleanup before quitting the application.

Quit from the top menu bar works fine and it raises 'close' event. Am I doing something wrong? Is there any other way to do cleanup when the user quits from the dock?

My Source Code:

var gui = require('nw.gui');
var splashWindow = gui.Window.get();

if (platform() == 'darwin') {
var macMenu = new gui.Menu({type: 'menubar'});
macMenu.createMacBuiltin('Test App');
splashWindow.menu = macMenu;
}

splashWindow.on('loaded', function() {
....
...
}

splashWindow.on('close', function() {

});

postgres-mirror pushed a commit to pgadmin-org/pgadmin4 that referenced this issue Feb 22, 2021
…its the

application from the top menu bar on OSX.

Python server will not be closed if the user quits the application from Dock,
for this we have updated the issue nwjs/nw.js#7365

refs #6244
@rogerwang rogerwang reopened this Feb 26, 2021
rogerwang added a commit to nwjs/chromium.src that referenced this issue Mar 9, 2021
@rogerwang
Copy link
Member

@Cubelrti
Copy link
Contributor

Cubelrti commented Jul 8, 2021

nwjs/chromium.src@594efa2
This commit seems to break SDK quit behavior like:

  • Right click to quit from Dock
  • Activity Monitor - Quit Process
  • Even shutting down will be blocked by application

Reverting this commit seems fix those issue and the application quit normally, could you investigate further on this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants