1540309 - Web Authentication - UI dialog follows focus inappropriately

Status: RESOLVED FIXED

(Core :: DOM: Web Authentication, defect, P1)

Description

[J.C. Jones [:jcj] (he/him)]

Attachment: wandering_dialog_syndrome.mov

Bug 1448408 reworks how visibility events are handled in Web Authentication, and it introduces a UI bug that's probably easy to figure out for someone skilled in the arcane arts.

WebAuthnPromptHelper [0] in browser.js controls this, but I'm not really sure how to fix it. My ham-fisted hacking approach isn't immediately successful, so I wanted to solicit feedback from the wizards.

[0] https://searchfox.org/mozilla-central/rev/532e4b94b9e807d157ba8e55034aef05c1196dc9/browser/base/content/browser.js#7157

Comment 1

[J.C. Jones [:jcj] (he/him)]

Milord Johann, glorious wizard of the Urstromtal, could you lend me your powers of diagnosis?

🧙‍♂️🔎

Comment 2

[Johann Hofmann [:johannh]]

Sorry for putting this off for so long, I've tried to look at this several times but I don't really understand the problem. Can you describe it in terms of expected vs. actual behavior?

Is this a regression from 1448408?

Thanks :)

Comment 3

[J.C. Jones [:jcj] (he/him)]

Maybe we should hop in a Vidyo sometime tomorrow?

But basically, the WebAuthnPromptHelper shouldn't move from one window to the other just by nature of clicking on another window. It should be associated with the window it was started from. It might have nothing to do with the second window.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Johann, are you intending to work on this?

Comment 5

[Patricia Lawless]

Per Johann on Slack: he has been busy with other urgent things and likely won't get to this one for Fx68.

Comment 6

[Johann Hofmann [:johannh]]

Sorry for the delay!

So, well, the reason this happens is because this whole thing is super brittle and was basically clinging for its life onto the expectations that nothing would ever change, specifically it is completely reliant on the fact that we immediately discard the prompt when leaving the foreground, which is what bug 1448408 got rid of.

Now, we send an observer notification across the parent process to all open windows, and those will happily show the prompt in their respective selected tab, because we don't have any information about which is the right tab to show the prompt for.

As a super dirty hack we could only show this in the currently visible window, but then what if a background tab tries to show a permission prompt?

This prompt is in a really bad state right now and I would wager that if we can't put resourcing on rewriting the prompting IPC we might need to backout bug 1448408 :/

Comment 7

[J.C. Jones [:jcj] (he/him)]

(In reply to Johann Hofmann [:johannh] from comment #6)

As a super dirty hack we could only show this in the currently visible window, but then what if a background tab tries to show a permission prompt?

On the upside, we really shouldn't allow background tabs or windows to use WebAuthn - hence the original purpose. However, we do need to let it persist through context switches where Firefox itself goes into the background...

Comment 8

[Johann Hofmann [:johannh]]

(In reply to J.C. Jones [:jcj] (he/him) from comment #7)

(In reply to Johann Hofmann [:johannh] from comment #6)

As a super dirty hack we could only show this in the currently visible window, but then what if a background tab tries to show a permission prompt?

On the upside, we really shouldn't allow background tabs or windows to use WebAuthn - hence the original purpose. However, we do need to let it persist through context switches where Firefox itself goes into the background...

By persist, do you mean it should also not hide into a small icon on losing focus? There's a "persist" flag for that, but you'll need to handle cancelling the prompt when the user is switching tabs then, right? (if yes then you would already need to do that with the current implementation)

Comment 9

[J.C. Jones [:jcj] (he/him)]

No, I'm sorry. I mean: when a window running WebAuthn loses focus, it should not cause cancellation of the WebAuthn promise. That was what bug 1448408 was about, so that we unbroke using other authenticators (Windows Hello, MacOS SoftU2F) which had to take focus from Firefox to do their job.

Comment 10

[Johann Hofmann [:johannh]]

Ok, the Promise part is clear, but what about the doorhanger visibility? If you leave the Promise dangling but hide the doorhanger when losing focus (which you currently do) that's actually the worst case for your consumers, because most users don't notice the minified doorhanger and websites have no way of telling whether the user switched focus or is just taking really long to decide.

Here's what I think you want to do:

• Make the permission prompt persistent (i.e. never hide)
• Cancel the permission request when it came from a background tab or inactive Firefox window or when the user switches away from the tab

Now you still have the issue that background windows can start permission requests and they will appear in the front window.

I start to think there's no way around rewriting the prompt code...

Comment 11

[J.C. Jones [:jcj] (he/him)]

(In reply to Johann Hofmann [:johannh] from comment #10)

Ok, the Promise part is clear, but what about the doorhanger visibility? If you leave the Promise dangling but hide the doorhanger when losing focus (which you currently do) that's actually the worst case for your consumers, because most users don't notice the minified doorhanger and websites have no way of telling whether the user switched focus or is just taking really long to decide.

Riightt... Yes, it's not ... good.

Here's what I think you want to do:

• Make the permission prompt persistent (i.e. never hide)
• Cancel the permission request when it came from a background tab or inactive Firefox window or when the user switches away from the tab

Yes, those are correct.

Now you still have the issue that background windows can start permission requests and they will appear in the front window.

Which isn't great, but as long as they only put a doorhanger / UX on their own window, I think that's livable.

I start to think there's no way around rewriting the prompt code...

:(

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

This is P1 and still unassigned, is it still something you think should be P1 ? Aiming to fix in 70?

Comment 14

[J.C. Jones [:jcj] (he/him)]

I certainly think it should be P1, but no resources to assign to it. No chance of any resources in the 70 timeframe, either.

I would have to learn how to (re)-write UX code for this, per Comment 10, were I to take it on.

Comment 15

[J.C. Jones [:jcj] (he/him)]

Bug 1630088 now provides a reasonably straightforward fix for this, I believe.

Comment 16

[J.C. Jones [:jcj] (he/him)]

Attachment: Bug 1540309 - Track browsing context for the WebAuthn UX prompts

The WebAuthn popup just used the current selected browser for its context,
while we actually want to stick with the window that brought us to the party.

Depends on D71702

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4b7e943b93c
Track browsing context for the WebAuthn UX prompts r=nhnt11

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/b4b7e943b93c