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: consistent launch on login menu item #33

Closed
wants to merge 4 commits into from

Conversation

JokerQyou
Copy link

@JokerQyou JokerQyou commented Feb 20, 2024

According to Apple's doc SMAppService.Status.notFound should only be returned when there's an error, but for some reason it is returned when the service is not registered. This PR treats notFound the same as notRegistered, and simplifies menu item state.

@sannidhyaroy
Copy link
Owner

sannidhyaroy commented Feb 20, 2024

I don't have my MacBook around right now to check this out but I did try to fix and improve the Launch on Login functionality on multiple commits in the past. This is the last such commit f5196da. Is it still not working?

@JokerQyou
Copy link
Author

JokerQyou commented Feb 20, 2024

As I commented in agungrbudiman/issues/9 , the "launch on login" works if I manually add Sudoto to login item list in System Settings. If the state is inconsistent, this menu item would not work - there's no way to turn it on or off. To trigger the issue:

  • Launch Sudoto and open the menu
  • If "launch on login" is checked, open System Settings and manually remove Sudoto from login item list. The menu item should break now, it would show "-" before the text.
  • If it is not checked, and there is no "-" before the text, click on it to turn on "launch on login". After it's checked, remove Soduto from login item list of System Settings.

Once the issue is triggered, the menu item does nothing when clicked.

This PR basically removed the "mixed" state from the menu item, and treat an error state as "notRegistered". It's not something urgent, feel free to test it when you have access to a Macbook. :)

@JokerQyou
Copy link
Author

JokerQyou commented Feb 20, 2024

This issue is caused by inconsistent state, and the current PR does not fully fix it. I think I fully understand what's going on now. The following content applies to code before applying this PR.

  1. Starting from the blank state: the user installs Soduto the first time, there's no data stored.
  2. The user clicks on "launch on login" menu item, Soduto registers itself as login item. Now Soduto saves launchOnLogin to 1. You can verify that by running defaults read com.soduto.Soduto. Relevant code:
    switch (loginItem.status.rawValue) {
    case 0:
    if ((try? loginItem.register()) != nil) {
    self.userDefaults.set(true, forKey: Property.launchOnLogin.rawValue)
    } else {
    self.userDefaults.set(false, forKey: Property.launchOnLogin.rawValue)
  3. The user manually remove Soduto from login item list in System Settings. Now the actual login item is gone, and further SMAppService.mainApp.status call would return notFound. But Soduto still has launchOnLogin state set to true in memory, it's also stored in userDefault state on disk. You can again verify that by running defaults read com.soduto.Soduto.
  4. The user opens Soduto menu. When the "launch on login" menu item renders, it would call SMAppService.mainApp.status again and get the actual login item state from macOS, see code here:
    if #available(macOS 13.0, *) {
    let loginItem = SMAppService.mainApp
    switch (loginItem.status.rawValue) {
    case 0:
    self.launchOnLoginItem.state = NSControl.StateValue.off
    break
    case 1:
    self.launchOnLoginItem.state = NSControl.StateValue.on
    break
    default:
    self.launchOnLoginItem.state = NSControl.StateValue.mixed
    break
    }
    } else {
    . This would return notFound, which hits the default branch, so the menu item would show as "mixed" state (with a "-" sign before text).
  5. When the user click on "launch on login" menu item again, it would call set method of launchOnLogin property with a newValue of false (since Soduto has launchOnLogin set to true currently). But A) For macOS >= 13.0, it does not perform action according to newValue (true for enabling "launch on login", false for disabling), but rather decide what to do based on current actual login item state. See here:
    switch (loginItem.status.rawValue) {
    case 0:
    if ((try? loginItem.register()) != nil) {
    self.userDefaults.set(true, forKey: Property.launchOnLogin.rawValue)
    } else {
    self.userDefaults.set(false, forKey: Property.launchOnLogin.rawValue)
    self.notification.ShowCustomNotification(title: "Uh'oh'", body: "We encountered a problem! Try toggling Soduto under Login Items manually", sound: true, id: "LoginItemOff")
    SMAppService.openSystemSettingsLoginItems()
    }
    break
    case 1:
    if ((try? loginItem.unregister()) != nil) {
    self.userDefaults.set(false, forKey: Property.launchOnLogin.rawValue)
    } else {
    self.userDefaults.set(true, forKey: Property.launchOnLogin.rawValue)
    self.notification.ShowCustomNotification(title: "Uh'oh", body: "We encountered a problem! Try toggling 'Soduto' under Login Items manually", sound: false, id: "LoginItemsOn")
    SMAppService.openSystemSettingsLoginItems()
    }
    break
    case 2:
    SMAppService.openSystemSettingsLoginItems()
    self.notification.ShowCustomNotification(title: "Uh'oh!", body: "macOS requires approval to let Soduto change login item settings. Tap the + icon and add 'Soduto' manually", sound: true, id: "LoginItemApproval")
    break
    case 3:
    print("SMAppService not found!")
    break
    default: break
    }
    . B) It would always hit case 3 branch (notFound == 3), so nothing will happen.

So to fully address this issue, we should either: A) have the login item state automatically synced into Soduto's memory (by using some kind of observable type in Swift), or B) update launchOnLogin value when the menu item renders.

@JokerQyou
Copy link
Author

I've updated the code. I think it fully resolves this issue, although the code is not optimal.

@JokerQyou JokerQyou closed this Mar 29, 2024
@JokerQyou JokerQyou deleted the fix/launch-on-login branch March 29, 2024 03:33
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 this pull request may close these issues.

2 participants