1892562 - Pressing a key in the Primary Password dialog causes a crashe in [@ -[ChildView keyDown:]] when focus manager test mode is enabled

Status: NEW

(Core :: Widget: Cocoa, defect, P5)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:127.0) Gecko/20100101 Firefox/127.0 ID:20240419213148

This crash happens all the time when any key is entered in the master password dialog. This can be triggered by opening a web page that requires the user to authenticate or by opening the preferences and checking Saved Passwords.

This is a regression, but I don't know since when at of this point. I could check for the range next week if needed.

Crash report: https://crash-stats.mozilla.org/report/index/d26b48f2-3983-44e2-af40-df0fe0240420

MOZ_CRASH Reason: MOZ_CRASH(A password editor has focus, but not in secure input mode)

Top 10 frames:

0  XUL  -[ChildView keyDown:]  widget/cocoa/nsChildView.mm:3623
1  AppKit  -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:]
2  AppKit  -[NSWindow(NSEventRouting) sendEvent:]
3  XUL  -[ToolbarWindow sendEvent:]  widget/cocoa/nsCocoaWindow.mm:3806
4  AppKit  -[NSApplication(NSEventRouting) sendEvent:]
5  XUL  -[GeckoNSApplication sendEvent:]  widget/cocoa/nsAppShell.mm:185
6  XUL  nsAppShell::ProcessNextNativeEvent(bool)  widget/cocoa/nsAppShell.mm:725
7  XUL  nsBaseAppShell::DoProcessNextNativeEvent(bool)  widget/nsBaseAppShell.cpp:131
7  XUL  nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool)  widget/nsBaseAppShell.cpp:267
7  XUL  nsAppShell::OnProcessNextEvent(nsIThreadInternal*, bool)  widget/cocoa/nsAppShell.mm:954

Comment 1

[Stephen A Pohl [:spohl]]

I'm unable to reproduce this so far. A regression range and a website that requires authentication that reproduces this would be helpful.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

This bug has been marked as a regression. Setting status flag for Nightly to affected.

Comment 3

[Dianna Smith [:diannaS]]

Unless there was a different signature before, looks like it started in build 20240415203932. Corresponding bug list

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This actually happens when the focus manager test mode is enabled. You can enable it by flipping the preference focusmanager.testmode to true in about:config. Once a Primary Password dialog is opened and you start typing Firefox crashes.

This works fine in Firefox 126.0b6. So it probably regressed with version 127.0.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Results from mozregression:

11:19.35 INFO: Narrowed integration regression window from [2bdd34aa, 2ed95f62] (3 builds) to [a8a5e65e, 2ed95f62] (2 builds) (~1 steps left)
11:19.35 INFO: No more integration revisions, bisection finished.
11:19.35 INFO: Last good revision: a8a5e65ed70cbe9363a43c3ec5b18920996ba59f
11:19.35 INFO: First bad revision: 2ed95f62c8ba239daadc33b7f909253ca318d2fd
11:19.35 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a8a5e65ed70cbe9363a43c3ec5b18920996ba59f&tochange=2ed95f62c8ba239daadc33b7f909253ca318d2fd

Emilio, can you please take a look?

Comment 6

[Emilio Cobos Álvarez (:emilio)]

It seems that in test mode, we don't focus the widget here, so this seems kind of expected.

Given the crash is nightly-only and this only happens on test mode, I don't think this is really worth too much investigation, but... Masayuki, this this seems to be related to bug 893973, do you think it's expected or worth addressing somehow?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I don't think this is S2 unless I'm missing something, users should never have testmode.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I didn't assume that somebody runs the test mode to type something. We enable/disable secure input mode when an editable element on focused widget gets focus. Therefore, it won't work well. The API call is stateful and IIRC, it's shared entire the system. Therefore, within the odd mode, we cannot enable/disable the secure input mode, e.g., when a password field in the background window, it should not enable the secure input mode. I think that WONTFIX may be reasonable, but if this blocks somebody's testing, I think that the assertion should check the pref value.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

With any Remote Protocol enabled (WebDriver, CDP) we enable the focus manager test mode. Given that we allow testing in chrome scope as well with WebDriver classic so far (BiDi to come) there can be situations when folks will trap into this situation.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #9)

With any Remote Protocol enabled (WebDriver, CDP) we enable the focus manager test mode. Given that we allow testing in chrome scope as well with WebDriver classic so far (BiDi to come) there can be situations when folks will trap into this situation.

Okay, then, if the pref is enabled and the automation mode is enabled, the assertion can relax (I guess nobody wants to protect typing password in the automation mode. (Or could be we should manage the secure input mode when first key down if in the test mode.)

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6) from comment #10)

Okay, then, if the pref is enabled and the automation mode is enabled, the assertion can relax (I guess nobody wants to protect typing password in the automation mode. (Or could be we should manage the secure input mode when first key down if in the test mode.)

Note that Remote Protocols need their own check if enabled. Is general isInAutomation flag doesn't work here.