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

Add option to automatically download/install new updates in permission prompt #2209

Closed
zorgiepoo opened this issue Jul 19, 2022 · 9 comments · Fixed by #2285
Closed

Add option to automatically download/install new updates in permission prompt #2209

zorgiepoo opened this issue Jul 19, 2022 · 9 comments · Fixed by #2285
Milestone

Comments

@zorgiepoo
Copy link
Member

zorgiepoo commented Jul 19, 2022

From discussion #2208, in the permission prompt I think we should consider adding a checkbox option that reflects SUAutomaticallyUpdate / -setAutomaticallyDownloadsUpdates: (i.e, an option to automatically download/install new updates).

Current permission prompt:
Screen Shot 2022-07-18 at 9 03 05 PM

@zorgiepoo zorgiepoo added this to the 2.4 milestone Jul 19, 2022
@core-code
Copy link
Contributor

does this also mean re-adding the checkbox from Sparkle 1.x? or is there already some way to get that one back?
Screenshot 2022-08-29 at 21 10 19

@zorgiepoo
Copy link
Member Author

That shouldn't have changed. This is for the permission prompt. That checkbox on the update prompt window in particular may not show up when automaticallyChecksForUpdates is NO or when SUAllowsAutomaticUpdates is NO (I think, or something like that, not looking at the code/docs at the moment, off top of my head)

@core-code
Copy link
Contributor

That shouldn't have changed

i just can't confirm that. the button was there with Sparkle 1.x and its gone in Sparkle 2.x - nothing was changed except upgrading the framework.

looking at the source code i can't see how it would work either:

_allowsAutomaticUpdates = updaterSettings.allowsAutomaticUpdates && updaterSettings.automaticallyChecksForUpdates && !item.informationOnlyUpdate;

so the button is only there if the user has already checked the button (huh!?) or if SUAllowsAutomaticUpdatesKey is true. but we don't want to set SUAllowsAutomaticUpdatesKey to true, since automatic updates should be off by default and only on if the user activated it himself. but the user can't that without seeing the button. chicken and egg :-)

i changed the line to

_allowsAutomaticUpdates = (updaterSettings.allowsAutomaticUpdates || updaterSettings.automaticallyChecksForUpdates) && !item.informationOnlyUpdate;

which seems to work fine for me.

@zorgiepoo
Copy link
Member Author

zorgiepoo commented Sep 16, 2022

A little misunderstanding I think but yes there's a potential issue.

_allowsAutomaticUpdates refers to allowing user to opt into automatic downloading/installing of updates.
automaticallyChecksForUpdates refers to Sparkle's automatic checking of new updates, which you seem to not use and have set to NO (?)
SUAllowsAutomaticUpdates defaults to true. It's for developers that want to set it to false that want to not provide the user the option to automatically download/install updates.

If automaticallyChecksForUpdates is set to NO, Sparkle doesn't know the developer wants to schedule update checks themselves (not always the case, sometimes a user/developer may not want to schedule automatic update checks at all..). In this scenario, providing an option to automatically download/installing updates is a bit unsound because automatic checking is NO (or 'unspecified' in this case of permission prompt (if kept enabled) not being requested yet).

However this also breaks, which you may have found out, if the developer wants to disable Sparkle's scheduler and check for updates themselves automatically, and also provide the user the option to opt into allowing automatic installing/downloading of updates. I think this is your case but let me know if I'm wrong.

--

I can think of a simple way to fix this. If SUAllowsAutomaticUpdates is specified, always use that value (except info only updates). If unspecified, use current behavior. So then you can specify SUAllowsAutomaticUpdates as YES and the user will have the option to toggle it.

@core-code
Copy link
Contributor

core-code commented Sep 16, 2022

and also provide the user the option to opt into allowing automatic installing/downloading of updates. I think this is your case but let me know if I'm wrong.

confirmed, this is exactly my case.

automaticallyChecksForUpdates refers to Sparkle's automatic checking of new updates, which you seem to not use and have set to NO (?)

i can confirm that. its set to no because we don't want Sparkle to "ask users on second launch for permission if they want automatic update checks enabled".

@zorgiepoo
Copy link
Member Author

Change on your end for that issue will be to set SUAllowsAutomaticUpdates to YES in Info.plist with #2266

@core-code
Copy link
Contributor

thanks! i'll try that, and hope it won't have any other consequences besides showing that checkbox :)

@zorgiepoo
Copy link
Member Author

zorgiepoo commented Sep 21, 2022

it's only for the UI to allow changing automatic downloading/installing updates setting and it's only used in the update alert for hiding/showing the control / managing some UI constraints (Sparkle 1 is slightly different, but setting it there to YES too has no consequential effect).

@zorgiepoo
Copy link
Member Author

Adding this change in #2285

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

Successfully merging a pull request may close this issue.

2 participants