Open Bug 1540309 Opened 1 year ago Updated 1 year ago

Web Authentication - UI dialog follows focus inappropriately

Categories

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

68 Branch
defect

Tracking

()

Tracking Status
firefox68 --- fix-optional
firefox69 --- wontfix
firefox70 --- wontfix

People

(Reporter: jcj, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

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

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

🧙‍♂️🔎

Flags: needinfo?(jhofmann)

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 :)

Flags: needinfo?(jhofmann) → needinfo?(jjones)

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.

Flags: needinfo?(jjones)

Johann, are you intending to work on this?

Flags: needinfo?(jhofmann)

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

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 :/

Flags: needinfo?(jhofmann)

(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...

(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)

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.

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...

(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...

:(

Duplicate of this bug: 1573194
See Also: → 1573190

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

Flags: needinfo?(jjones)

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.

Blocks: 1573190
You need to log in before you can comment on or make changes to this bug.