1735076 - No indication of button focus when left and right arrow (cursor) keys are used in `Save Message` confirmation prompt

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect, P1)

Description

[K Moser]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

1. Open Write dialog (File->New->Message) and begin to write a message.
2. Close window to get the "Save Message" modal to pop up.
3. Left- and right-arrow keys are inoperative, i.e. don't cycle between Save, Discard changes, and Close options as expected in any standard Windows native modal.

Actual results:

Arrow keys don't work to cycle between options; only tab key works.

Expected results:

Save, Discard changes, and Close options should have been highlighted as I cycled between them, just as with the tab key.

Comment 1

[Wayne Mery (:wsmwk)]

Indeed, tab works, arrow does not.

Thomas, is this by design?

Comment 2

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #1)

Indeed, tab works, arrow does not.
Thomas, is this by design?

Hell no! And it's much worse and more cunning than described: The cursor keys are functional, but there's zero visible indication that the focus has moved to another button. So if for whatever reason I change my mind and want to save the message anyway, invisible focus from arrow keys might be on Discard..., and Enter will discard, but Save is the only button showing blue. Maybe it's this modern thing of showing focus ring only when keyboard is used, which fails to kick in here. Tab shows the correct behaviour.

This is basic functionality seriously broken for keyboard use, and with unexpected danger of dataloss. Maybe a toolkit thing, so probably affects other dialogs, too.

Alex, can you make a plan or otherwise ask someone else?
Richard, any insights?

STR (on Windows 10, 91.4.0 (64-bit))

1. compose msg
2. close msg (Alt+F4, or [x] button)
3. On Save message dialog, press arrow right exactly one time (sic! Ignore that there's no visual feedback per this bug! Do not press Tab before you press arrows)
   This assumes the button sequence on Windows: Save, Discard changes, Cancel. Your OS might have different order, or bug may not occur on other OS.
4. Press Enter

Actual

• After step 3, focus has correctly moved to Discard... button, but there's zero indication of the focus change, so only Save default button looks focused
• Ater step 4, Discard... button is unexpectedly triggered even though it did not have any visible indication of focus, and I actually wanted Save!
• Dataloss! Message discarded against my will.
• Using arrow keys is now Russian roulette as user will never see which button really has focus.

Expected

• Using arrow keys must show and move the focus ring on buttons, not just move the focus without visible indication
• prevent unexpected dataloss and random unpredictable behaviour

Comment 3

[Thomas D. (:thomas8)]

Does anyone see this on Linux or Mac?

Comment 4

[Thomas D. (:thomas8)]

After pressing Tab once (which shows the focus ring), this bug will no longer occur on the current dialog. Which seems to confirm my theory that cursor keys are somehow failing to trigger unhiding the focus ring which is only shown when keyboard gets used.

Comment 5

[Richard Marti (:Paenglab)]

The same happens on Linux and Mac.

It's not a theme issue as with TAB the focus works. The arrow key move doesn't activate the focus show. When first using TAB and then the arrow keys the focus ring shows and moves until it reaches the end. Pressing the arrow key over the end of the chain disables the focus and nothing is shown further.

Comment 6

[Alessandro Castellani [:aleca]]

Uh, good find, this is indeed super weird.
Pinging emilio to see if he knows of any related m-c issue, and if he can redirect us to someone that manages that area to see if it's a toolkit issue, or an implementation issue on our end.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1735076 - Use ProgrammaticFocusFlags from nsXULCommandDispatcher. r=smaug

Relevant calls are here:

https://searchfox.org/mozilla-central/rev/21a9b72545da06681db97c4b3a2a6be761f4aae5/toolkit/content/widgets/button.js#42-68

This makes sure to set the FLAG_BYKEY properly if needed, instead of passing
down raw flags to nsFocusManager. Clean up a bit while at it.

Let me know if you want a test for this, but we have tests for programmatic
focus and :focus-visible already, so my gut feeling is that testing this
particular XUL-specific change is not super-worth-it...

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de1608e5f69b
Use ProgrammaticFocusFlags from nsXULCommandDispatcher. r=smaug

Comment 9

[Atila Butkovits]

Backed out changeset for causing failures at browser_searchbar_openpopup.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/9853a5b7bc55ed63e7cd586125ee43cc9f8828f4

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=UBR5hp03S0i9SM-iZCahfQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=de1608e5f69b6f0906d09ce3b1471e9571ec08ad

Failure log: https://treeherder.mozilla.org/logviewer?job_id=361138283&repo=autoland&lineNumber=5332

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93b9da069dde
Use ProgrammaticFocusFlags from nsXULCommandDispatcher. r=smaug

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out changeset 93b9da069dde (Bug 1735076) for causing failures in browser_searchbar_openpopup.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=93b9da069dde682f4a95098b863cd56bb3e31f1b&selectedTaskRun=TSXMxQhzRqWm15jR7vTcdw.0

Log: https://treeherder.mozilla.org/logviewer?job_id=362037134&repo=autoland&lineNumber=3231

Backout: https://hg.mozilla.org/integration/autoland/rev/1488c9d35203c0897ef425ab8196a4e7a7b9ff22

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a2735991204
Use ProgrammaticFocusFlags from nsXULCommandDispatcher. r=smaug

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/0a2735991204