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

prevent focus stealing of update notification #2292

Closed
1 task done
crackwitz opened this issue Aug 18, 2019 · 14 comments
Closed
1 task done

prevent focus stealing of update notification #2292

crackwitz opened this issue Aug 18, 2019 · 14 comments
Assignees

Comments

@crackwitz
Copy link

crackwitz commented Aug 18, 2019

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options

git version 2.23.0.windows.1
cpu: x86_64
built from commit: 4db2e5cc9e1522131a039cbad3970f147a39f0ce
sizeof-long: 4
sizeof-size_t: 8
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver

Microsoft Windows [Version 6.1.7601]
  • What options did you set as part of the installation? Or did you choose the
    defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: Nano
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: ConHost
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Disabled
Enable Builtin Interactive Add: Disabled
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

None.

Details

None. It's the daily update check.

  • What did you expect to occur after running these commands?

That it appears, but does NOT steal focus. The proper thing to do is to flash the task bar icon/button for attention, but otherwise not disrupt the user.

  • What actually happened instead?

It stole the focus. If I had been typing at the moment and didn't notice, I would have eventually hit space bar and thus pressed some button in the dialog.

@PhilipOakley
Copy link

Are you able to have a look at the build-extra repo that creates that facility and see if you can pick out the relevant parts of the repo, at least as a helper to others to get them started.

If you report back what you find then other contributors can work with you ensure that the right problem is fixed.

If you find that you get all the way to a fix, then a Pull Request would also be welcomed.

@rimrul
Copy link
Member

rimrul commented Aug 19, 2019

I don't see a way to tell MessageBoxW() to not take the keyboard focus. Maybe this can be worked around by calling GetForegroundWindow() before MessageBoxW() and SetForegroundWindow() afterwards.

Edit: That won't work as MessageBoxW() only returns after closing the MessageBox. Maybe LockSetForegroundWindow()

Edit2: If that doesn't work I think we can only set the MB_DEFBUTTON2 flag to make the default focussed button 'No'.

@crackwitz
Copy link
Author

I saw that a custom program is talked about which supersedes the tcl/tk way. a simple call to MessageBoxW might not be enough. Windows GUI does have ways of solving this: https://stackoverflow.com/questions/16209121/how-do-i-prevent-showdialog-from-stealing-focus-from-other-applications

@dscho
Copy link
Member

dscho commented Aug 19, 2019

Wait. The real issue here is why the update notification is not shown as a Toast, but as a dialog box. Has anybody looked into that issue?

This is the call that should show the toast: https://github.com/git-for-windows/build-extra/blob/7d7daddea1701455fe589eb45ba6b02297369d72/git-extra/git-update-git-for-windows#L229-L233

And this seems to be what you're seeing: https://github.com/git-for-windows/build-extra/blob/7d7daddea1701455fe589eb45ba6b02297369d72/git-extra/git-update-git-for-windows#L264-L269

I say "seems" because it is not really clear to me what is happening.

@rimrul
Copy link
Member

rimrul commented Aug 19, 2019

There isn't any TCL/TK involved anymore since March 1st. It was changed to a C implementation for accessibility reasons. It uses WinAPI to create a MessageBox. The solution from the StackOverflow link you posted would require a .NET/winforms based implementation with a custom dialog.

@crackwitz
Copy link
Author

since "Toast" is only available on Win10 (maybe earlier, certainly not Win7), maybe this problem will "go away" when people stop using Win7 and Win8/8.1.

I'm saying to not use MessageBoxW but create a custom window/form and control its creation flags. that doesn't require C#, only plain old winapi in C. it is more work though.

@rimrul
Copy link
Member

rimrul commented Aug 19, 2019

All of that Stackoverflow discussion is about System.Windows.Forms.Form.Show().
I'm sure it's possible with WinAPI, but that Stackoverflow link has nothing to do with it.

@mfriedrich74
Copy link

mfriedrich74 commented Aug 19, 2019 via email

@rimrul
Copy link
Member

rimrul commented Aug 19, 2019

Yes, Windows 7 (and older versions, too) allows you to create tray notifications, too. they aren't quite toast messages, but close enough for many intents and purposes. The "WinToast" tool used by Git for Windows only works on Windows 8 and Windows 10. Also those tray notifications don't allow you yes/no buttons AFAIK, thus aren't what's wanted for the updater. With WinToast, this only works on Windows 10.

@crackwitz
Copy link
Author

crackwitz commented Aug 19, 2019

so, is focus stealing on windows 7 a concern? I can see why it wouldn't be, seeing as Win7 will lose support from MS soon.

if it is though, solutions could be:

  • nag "WinToast" to implement support for Windows 7
  • create a notification (possibly repeatedly/shortly after login) that needs to be clicked before it starts the updater (I don't see a reason why the user has to answer yes/no when ignoring the notification is a valid choice)
  • custom dialog with WinAPI in C that uses whatever creation flags necessary to avoid claiming focus
  • if plain WinAPI is too bothersome, go from C to C# for this utility program, and use WinForms or WPF for a custom dialog

@dscho
Copy link
Member

dscho commented Aug 22, 2019

is focus stealing on windows 7 a concern?

Yes.

* nag "WinToast" to implement support for Windows 7

A better alternative would be for you to offer a Pull Request instead.

This would be the option I prefer, by far.

@crackwitz
Copy link
Author

I'm just a user. I am entirely unfamiliar with the code base of wintoast or of git-for-windows. don't put your hopes in me.

@PhilipOakley
Copy link

hi @crackwitz any help is always welcome, even if it is only a small step. Git is open source so we should be able to have lots of small incremental contributions from the millions of Windows downloads...

Even just the step of being able to special case your Microsoft Windows [Version 6.1.7601] may get you enough along the road to allow an adequate resolution for the older installs (E.g. you idea of flashing the icon, ..]

@dscho
Copy link
Member

dscho commented Aug 23, 2019

I'm just a user.

14 years ago, I was also just a user of Git. For me, also, there was something that did not quite work for me. I learned how to fix it, and how to contribute, and it was fixed. You can do the exact same thing, you just have to allow yourself to do it.

I am entirely unfamiliar with the code base of wintoast or of git-for-windows.

Don't be afraid to ask for help getting started.

don't put your hopes in me.

Oh, I don't. You might want to put your hopes in yourself, though, as it is unlikely that anybody else will work on this.

The fact is that this is Open Source. You have an enormous amount of power and control over your own fate. You can learn just enough C/C++ to fix this. And then fix it. It's quite a freedom you have there, not having to depend on, say, a company to fix this for you.

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

No branches or pull requests

5 participants