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

fix(nsis): should close app when Silent and ONE_CLICK #6100

Merged
merged 1 commit into from Jul 29, 2021

Conversation

@lwintermelon
Copy link
Contributor

@lwintermelon lwintermelon commented Jul 26, 2021

Currently, if uninstall When Silent is true and ONE_CLICK is defined, the uninstaller will try to unisntall the app without closing it, which cases problems if uninstall it sliently when the app is running, the uninstaller can't remove all files, so left a broken app dir.

How to reproduce:
create an app, set "oneClick": true in config, make a NSIS installer, install it and try Uninstall {appName}.exe /S to silently uninstall it when the app is running.
Actual behavior: see above description.
Expected behavior: close the app then do the uninstall.

@changeset-bot
Copy link

@changeset-bot changeset-bot bot commented Jul 26, 2021

⚠️ No Changeset found

Latest commit: a5c6e40

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Loading

@lwintermelon lwintermelon marked this pull request as draft Jul 26, 2021
@lwintermelon lwintermelon marked this pull request as ready for review Jul 26, 2021
@mmaietta
Copy link
Collaborator

@mmaietta mmaietta commented Jul 28, 2021

Just to confirm, did you test locally this fix with the various installer configurations? 🙂 A change like this wouldn't get caught in any unit tests (not sure if we can even create tests for nsis (un)installer scripts)

Loading

@lwintermelon
Copy link
Contributor Author

@lwintermelon lwintermelon commented Jul 28, 2021

As seen in the code, it only affects whether u enable oneClick in config and whether u uninstall the app sliently or not, so i have tested the four posssible combined cases locally.

As for the test, it's not easy, as (un)installer may need user Interaction,so may need UI automation tools.

Loading

@mmaietta mmaietta merged commit baf640d into electron-userland:master Jul 29, 2021
12 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants