1788698 - Input focus not updated properly when type attribute changes

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Comment 1

[Issam Mani [:issammani]]

While working on 1767250, we discovered that when the type attribute is mutated onclick from button to text we end up in an undefined focus state, where :focus-visible is not applied but the element is focused.
For context, here is a codesandbox with a minimal reproduction of the issue. When clicking the button, the resulting text input should have the outer focus ring.

Comment 2

[Sergey Galich [:serg]]

I suspect this belongs to Core::Widget, please move back if I'm wrong

Comment 3

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

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Stephen A Pohl [:spohl]]

(In reply to Issam Mani [:issammani] from comment #1)

While working on 1767250, we discovered that when the type attribute is mutated onclick from button to text we end up in an undefined focus state, where :focus-visible is not applied but the element is focused.
For context, here is a codesandbox with a minimal reproduction of the issue. When clicking the button, the resulting text input should have the outer focus ring.

If :focus-visible isn't applied but the element is focused, it seems to indicate that the widget-level is working correctly but the front-end fails to apply :focus-visible. This is even more likely if this reproduces on all platforms.

I can't think of the best component for this, so I'm going to assign it to Firefox > General for further triage.

Comment 5

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

Emilio could know something about this.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case from comment 0

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1788698 - Force focus rings when focused input switches to text control. r=smaug

Seems somewhat reasonable to do this, if we wanted to, since
nsFocusManager also makes these always match :focus-visible.

If you think it's worth doing I can add a test quite trivially.

Comment 8

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:emilio, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Need to write a test.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9df937f8890
Force focus rings when focused input switches to text control. r=smaug

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36440 for changes under testing/web-platform/tests

Comment 12

[Sandor Molnar[:smolnar]]

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

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot