z------------- / CPod

(UNMAINTAINED) A simple, beautiful podcast app for Windows, macOS, and Linux.
Apache License 2.0
745 stars 75 forks source link

Crash on reopening window on macOS #154

Open eralpsahin opened 5 years ago

eralpsahin commented 5 years ago

CPod version: v1.27.1 and master branch

Platform: This bug only applies to macOS.

Install type app and development install on Mac

Description: Right now, The "window-all-closed" event does not quit the app fully when it detects darwin. When the window is closed, user can either click on the app in dock to open a window again to be able to use the app or right click and quit the app.

Trying to open the window back again results in an error because "activate" event handler calls createWindow() without any arguments and the program crashes.

That event is specific to macOS so other platforms do not have this error. This is when the taskbarClose setting is unchecked.

If that setting is checked, then after closing the window and clicking on the app icon on dock to reopen the window, nothing happens because "activate" event does not handle window being not null.

I fixed these in this commit I can make a pull request. But, I think there is a deeper problem here. "window-all-closed" does not let the app quit even though the taskbarClose is unchecked. I think taskbarClose and that event should be changed so that there is no conflict.

There are other bugs related to this so I did not want to make a pull request without discussing first.

For example the menu for macOS gets created but app.dock.SetMenu() is not called so the default one is used. And without user_settings.json file the fields of settings object in the main.js file such as taskbarMinimize are undefined, they don't get initialized to the defaults from cbus-settings.js. This is somewhat related to fixing the crash on macOS.

z------------- commented 5 years ago

If I'm understanding this correctly, the better solution would be to properly load default settings in main.js and in the window-all-closed handler make the call to app.quit() conditional on taskbarClose rather than process.platform?