feat: move hello to foreground once window exists

[jonaskuske]

This checks for the existence of the Windows Hello window (className Credential Dialog Xaml Host) and moves it to the foreground once it is found.

Not the optimal solution yet, as this happens only once per auth process. If the authentication fails (which sometimes happens for whatever reason, #1), the Hello prompt is recreated behind the Terminal and this time stays there as the FocusHelloWindow task has already returned.

A more comprehensive solution would require some form of AutomationEventHandler or usage of the Win32 window hook API, but that's nothing I can dig into right now and even without that, this is already a massive UX improvement.

fix #4

[jonaskuske]

Alright, I also got a version working that, once it moved the Hello window to the foreground, keeps checking every second if it's still in the foreground and if not puts it there again. But

• I couldn't verify yet that it actually works when Hello errors because Hello auth didn't fail me yet today 😅
  (it does work however when you manually move focus back to the Terminal) Will report back later.
• I'm not sure if this is actually desired behavior? I don't think there's a situation where you'd intentionally want to move focus away from the Hello popup without confirming/cancelling it, but not sure, it's definitely quite "pushy" behavior.
  (but so are e.g. UAC popups so it should be ok imo)

What you you think @nullpo-head ?

[nullpo-head]

Hi Jonas! Thanks a lot for your big contribution again!!

I couldn't verify yet that it actually works when Hello errors because Hello auth didn't fail me yet today 😅

me neither, but I think we can get the feedbacks from the users in #4.

it's definitely quite "pushy" behavior.

I agree. Why don't we just setting it foreground only once? Do you have any cases where it is not enough?

[jonaskuske]

Do you have any cases where it is not enough?

The issue described in #1, which (unfortunately) happens quite regularly for me, too:
You press "OK" on the Hello prompt, the window disappears but immediately re-appears again with an error message because the face unlock failed for whatever reason. (and keeps failing until you switch to PIN and back to Face Unlock once)

Without the continuous checks, the re-appearing Hello prompt appears in the background, so you got super confusing behavior:
You click OK, Face Unlock disappears, you think you've authenticated successfully but instead the Face Unlock window just hid in the background, which takes some time to realize.

If instead we keep checking the window until the auth is complete, the re-appeared Unlock prompt and its error message are visible in the foreground and you know what's happening. And the only downside is that you can't intentionally let the pending unlock window linger in the background – for normal usage nothing changes, Hello appears, you hit OK/Cancel, done.

[jonaskuske]

Either way, any extension to fix the window in the foreground can and should be part of a separate PR, no need to discuss that before even the basic move-to-foreground functionality is added. This can be merged imo :)

But I'm afraid I can't make the same contribution to @BlackHoleFox's Rust implementation of the HelloAuthenticator as I really don't know a lot about Rust, and especially not how to achieve something akin to C# Tasks in Rust/Win32 :(

[BlackHoleFox]

Looks like your comment got duplicated, @jonaskuske :rip:

The changes here look somewhat simple so it wouldn't be hard to bring this to Rust. Biggest difference would be a background thread to replace the C# task, but that'd just be something like this. Everything else can come from the windows or windows-sys crate.

[nullpo-head]

You press "OK" on the Hello prompt, the window disappears but immediately re-appears again with an error message because the face unlock failed for whatever reason. (and keeps failing until you switch to PIN and back to Face Unlock once)

Without the continuous checks, the re-appearing Hello prompt appears in the background, so you got super confusing behavior:

I see. It makes sense.

[nullpo-head]

@jonaskuske It looks this does not work on Windows 11. Even if I move the Window Hello's dialog to the background, it was not moved to the foreground. Do you have any idea why? The window title seems the same, but I'm not sure.
The output of winlister, though I'm not sure what is the best way to get the window's title.

Windows Security	Yes	(984, 586)	(912, 801)	003D082E	Credential Dialog Xaml Host	No	No	No	Yes	00003F24	00001210	C:\Windows\System32\CredentialUIBroker.exe	Microsoftｮ Windowsｮ Operating System	Credential Manager UI Host	10.0.22000.1 (WinBuild.160101.0800)	Microsoft Corporation	

[nullpo-head]

BTW, could you rebase your branch to the latest master, please? I've made a change to CI so that it builds the release asset for pull requests for easier review.

[jonaskuske]

It looks this does not work on Windows 11

Oh, that's too bad! Unfortunately I don't have a W11 device to test on... Maybe now's the time to update my SB2?
And yeah, the window className seems to be the same, so not sure what causes this.

But anyway, moving the window to the background manually won't work. I didn't commit the "pushy" implementation that stickies the window in the foreground (though I can if you want), it just moves it there once.

[nullpo-head]

I didn't commit the "pushy" implementation that stickies the window in the foreground (though I can if you want), it just moves it there once.

Ah, I missed that. I'm sorry for that. I now think the both of "pushy" and non-pushy make sense. So, if you're ok, I proceed with merging the current commit. If you want to bring back the behavior, I'll merge this after you make that change. I tested the change on my environment, too. Everything seems to work fine. Thanks for your contribution!

[jonaskuske]

See #29, maybe we can get it to work on Windows 11 with one of the methods I tried there. If none of them work, sure, merge this one and we have Windows 10 support at least! :)

I now think the both of "pushy" and non-pushy make sense.

Also, ever since using my version here, I haven't seen the random Hello error a single time, so maybe moving the window to the foreground somehow fixes it and thus also makes the pushy behavior obsolete? 🙏 I'd keep it non-pushy for now and maybe add the pushy behavior later, if others still experience the problem even after this is merged.

[nullpo-head]

OK, let me merge this then. Thanks!

[Luttik]

I just tested locally and it seems to work 👍

[danepowell]

maybe we can get it to work on Windows 11 with one of the methods I tried there

This works for me on Windows 11. YMMV 😄

[just4747]

Is there a way to implement this fix ona normal Win11 install or is this just a self-contained/testable type thing for special dev environments?