sindresorhus/electron-util

enforceMacOSAppLocation doesn't work if the old app is currently open

fabiospampinato opened this issue · 8 comments

fabiospampinato commented

enforceMacOSAppLocation fails in this scenario:

  • Have Foo v1 located in /Applications/Foo.app
  • Open it
  • Have Foo v2, still named Foo.app, located somewhere else
  • Open it
  • It will ask the user to move it into /Applications
  • Instead of doing that, enforceMacOSAppLocation will focus to the currently open Foo v1 window

I suppose the actual moving of the .app file failed silently but this library was stil trying to open the app and the previously opened one gained the focus this way?

I guess in this situation another dialog should be shown asking the user to close the old app and retrying later.

sindresorhus commented

Maybe it should automatically close any running instances of the app before moving?

fabiospampinato commented

Maybe 🤔. Some scenarios:

  • If the app doesn't properly save its state on exit this may cause data loss => but maybe this package should be optimized for properly written apps anyway.
  • If the app is something like Skype maybe we don't want to kill the current videoconference => but who updates Skype while videoconferencing.
sindresorhus commented

Hmm, yeah. It's such an edge-case anyway. We can just show a prompt asking the user to close the existing running instance of the app?

fabiospampinato commented

Yeah I think if after the user clicks "Move to Applications folder" we can't move the app because there's another one with the same name running from /Applications we could display a second dialog that looks like this:

Close other instances

There are other instances of Foo running, you have to close them before moving Foo to the Applications folder.

Quit Foo                                     Close Others and Retry

Should I submit a PR for adding this?

sindresorhus commented

Yes. PR welcome.

fabiospampinato commented

It looks like part of this is an upstream bug: electron/electron#18805

moveToApplicationsFolder should throw under this scenario while instead it returns true.

sindresorhus commented

The mentioned issue was fixed in Electron v7: electron/electron@f6a2970

I guess we need to implement a custom conflict handler?

fabiospampinato commented

@sindresorhus It wasn't really fixed in v7, i.e. I think moveToApplicationsFolder still returns true instead of throwing an error even though it fails, but now we can show a meaningful error message when the app can't be moved for this reason. I'll submit a PR 👍