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

SetSystemTrayMenu doesn't work when run in goroutine #5039

Closed
2 tasks done
williambrode opened this issue Jul 29, 2024 · 10 comments
Closed
2 tasks done

SetSystemTrayMenu doesn't work when run in goroutine #5039

williambrode opened this issue Jul 29, 2024 · 10 comments
Labels
unverified A bug that has been reported but not verified

Comments

@williambrode
Copy link
Contributor

williambrode commented Jul 29, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

I changed my code so it would set the System Tray in a goroutine after another condition was met. The app still showed the system tray icon but right-clicking it did nothing. Moving it back so SetSystemTrayMenu is always run before App.Run() appears to fix the problem. It may be fine that this is a requirement - but its not documented anywhere and it would be good to at least return or log an error if SetSystemTrayMenu doesn't actually work in this case.

How to reproduce

See example code. Run SetSystemTrayMenu in a goroutine while App.Run() is executing.

Screenshots

No response

Example code

fyneApp := app.NewWithID("TestApp")
win := fyneApp.NewWindow("Master")
win.SetMaster()
go func () {
  time.Sleep(1 * time.Second)
  trayMenu := fyne.NewMenu("TestMenu",
			fyne.NewMenuItem("Test", func() {
				win.Show()
				fmt.Println("test"
			})}
  if desk, ok := fyneApp.(desktop.App); ok {
    desk.SetSystemTrayMenu(trayMenu)
  }
}
fyneApp.Run()

Fyne version

2.4.5

Go compiler version

1.22

Operating system and version

Windows 10

Additional Information

No response

@williambrode williambrode added the unverified A bug that has been reported but not verified label Jul 29, 2024
@andydotxyz
Copy link
Member

App.Run will block until the app exists. You should set up the system tray code before calling the Run() method.

@williambrode
Copy link
Contributor Author

williambrode commented Jul 30, 2024

Hi @andydotxyz! The core of this issue report was the following:

It may be fine that this is a requirement - but its not documented anywhere and it would be good to at least return or log an error if SetSystemTrayMenu doesn't actually work in this case.

I wasn't able to debug the issue I was having and had to binary search changes to find the reason our app tray stopped working. So I'm trying to suggest how to help future devs (some kind of error reporting).

Should I change the description to "No error when setting system tray fails"?

@andydotxyz
Copy link
Member

This is not a system tray requirement - it is how the GUI app lifecycle works. Documented in getting started at https://docs.fyne.io/started/apprun.

If the documentation needs to be made clearer then we can do that, but I'm not sure I understand why this is a system tray issue. Just like if you create another window after calling Run that window won't visibly appear because its creation will execute as the app shuts down...

@andydotxyz
Copy link
Member

I was having and had to binary search changes to find the reason our app tray stopped working.

Perhaps this was an important point that I overlooked... are you saying that somehow running the app and then setting up a system tray had worked in the past? I cannot understand how this is possible.

@williambrode
Copy link
Contributor Author

Every instance of a developer doing something incorrect with fyne could be excused as "they should know better". But I suggest that the library itself would be improved if it helped a developer see what they did wrong (through some kind of indication of an error). I've been coding for quite some time and this missed error handling seemed particularly bad to me, so I thought I'd try to help by letting you guys know. Maybe its too low priority to improve in the near future, but at least it might aid another dev who notices their system tray doesn't work and searches the issues for help (as I did).

The use case here is that we have system tray options that only work after login. So I was trying to set the system tray after the user logged in - which had to be after App.run() was called.

@andydotxyz
Copy link
Member

We are talking past each other here I think. I don't disagree that we should do everything we can to make this easy to understand. Which is why we spend so long on documentation and examples like the one I linked.

In this case I have no idea what we could possibly do with error handling because there has been no error. The code you are referring to has not run when you are expediting us to provide errors - everything after "Run" happens when the app is shutting down.

As GUI apps are event based or at least have multiple threads of execution the system tray info should be set when your user has logged in as you say. The time this happens is not due to the order of lines inside the main function...

I know I'm not managing to explain this well but I am not sure how else to illustrate the situation.

@williambrode
Copy link
Contributor Author

Perhaps the confusion here is that you didn't see that the tray app is set in a goroutine, while Run() is still running? I'm not running it after App.Run() returns.

At the very least App.Run() could set a flag saying "I'm currently executing" and SetSystemTrayMenu could simply check that flag and return an error if its already executing - right?

@williambrode williambrode changed the title System Tray app doesn't work when run after App.Run() SetSystemTrayMenu doesn't work when run in goroutine while App.Run() is running Jul 31, 2024
@williambrode williambrode changed the title SetSystemTrayMenu doesn't work when run in goroutine while App.Run() is running SetSystemTrayMenu doesn't work when run in goroutine while App.Run() is running Jul 31, 2024
@williambrode
Copy link
Contributor Author

I updated the title - I see how my original description could be interpreted as a simple code ordering problem.

@andydotxyz
Copy link
Member

Thanks for the clarification, I understand now.

I doubt very much that it has anything to do with the co-execution of "App.Run()" and may point to the system tray having to execute initial setup on the main thread. (i.e. not a goroutine)

If that is the case then we could fix internally to do the heavy lifting so you can call it from any thread.

@andydotxyz andydotxyz reopened this Jul 31, 2024
@andydotxyz andydotxyz changed the title SetSystemTrayMenu doesn't work when run in goroutine while App.Run() is running SetSystemTrayMenu doesn't work when run in goroutine Aug 6, 2024
andydotxyz added a commit to andydotxyz/fyne that referenced this issue Aug 6, 2024
@andydotxyz
Copy link
Member

Fixed on develop and will merge to v2.5.1 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unverified A bug that has been reported but not verified
Projects
None yet
Development

No branches or pull requests

2 participants