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

MacOS: Only activate after the application has finished launching #1903

Merged
merged 4 commits into from
Apr 29, 2021

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Apr 6, 2021

  • Tested on all platforms changed
    • MacOS 10.14.6
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This fixes the main menu not responding until you refocus, at least from what I can tell - though we might have to do something similar to linebender/druid#994 to fix it on Catalina.

Additionally, activating the application when it has launched means we don't need the activation hack from #1318 any longer - though I would like @francesca64 to give their input here, it's very likely that I'm wrong on this point!

@ArturKovacs ArturKovacs self-requested a review April 6, 2021 16:02
@francesca64 francesca64 self-requested a review April 6, 2021 21:21
@ArturKovacs
Copy link
Contributor

I tested this (I was moving my mouse while running cargo run --example multiwindow) and only one of the windows show up, so it seems this unfortunately does not replace the activation hack.

@madsmtm
Copy link
Member Author

madsmtm commented Apr 28, 2021

Oops, forgot to reply to this, but the discrepancy was indeed nicely spotted! I too will have to spend some more time on this / on #1918 at some point 😉

@madsmtm
Copy link
Member Author

madsmtm commented Apr 29, 2021

So, I found yet another hacky way to activate the windows, so this now works on the multiwindow example - however, the hack is much less involved, it only fixes the case where windows are created before applicationDidFinishLaunching and everything else works without it.

It should be noted that it could be implemented differently: By tracking windows created before applicationDidFinishLaunching instead of querying information about the windows later on. The chosen approach seemed simpler to me, but it might have some downsides e.g. if more complex things that are technically also windows also are created in this time frame

@madsmtm
Copy link
Member Author

madsmtm commented Apr 29, 2021

Anyway, my proposal going forward would be to merge this, with or without the activation hack (somebody other than me should decide whether we want to encourage creating windows before applicationDidFinishLaunching), and then rebase the setActivationPolicy movement from #1918 and let that fix it for MacOS 10.15 Catalina and above.

madsmtm added 4 commits April 29, 2021 15:35
This fixes the main menu not responding until you refocus, at least from what I can tell - though we might have to do something similar to linebender/druid#994 to fix it fully?
You can't make hidden windows the key window
For activating multiple windows created before the application finished launching
@madsmtm madsmtm force-pushed the fix-macos-activation branch from 148a8d1 to c3e11b3 Compare April 29, 2021 13:36
@ArturKovacs
Copy link
Contributor

I don't want to jinx it, but @madsmtm, I think you are a genius. It works perfectly.

@ArturKovacs ArturKovacs merged commit 2775156 into rust-windowing:master Apr 29, 2021
@ArturKovacs
Copy link
Contributor

I somewhat hastily merged this, but @adamnemecek could you test if this works for you?

@madsmtm
Copy link
Member Author

madsmtm commented Apr 29, 2021

Well, it shouldn't work for them (and everyone else on Catalina or above), it's missing #1922, right?

@ArturKovacs
Copy link
Contributor

All that #1922 does is that it allows you to specify an activation policy other than NSApplicationActivationPolicyRegular. So if I'm not mistaken than if this doesn't work on 10.15, then #1922 won't work either.

@casperstorm would mind testing this to see if the default menu can be opened right after startup, on the master branch?

@madsmtm
Copy link
Member Author

madsmtm commented Apr 30, 2021

The vital thing is, it also moves the setting of the activation policy to applicationDidFinishLaunching, where previously it was called before, and apparently that causes problems on Catalina and above.

@casperstorm
Copy link
Contributor

casperstorm commented Apr 30, 2021

@casperstorm would mind testing this to see if the default menu can be opened right after startup, on the master branch?

It does open right after startup on 10.15.7 👍🏻

@ArturKovacs
Copy link
Contributor

Awesome, thank youuu 😊

@madsmtm madsmtm deleted the fix-macos-activation branch June 5, 2021 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants