1818287 - form associated custom elements with [disabled] do not fire events

Status: RESOLVED FIXED

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

Description

[clshortfuse]

Steps to reproduce:

• Create FACE Element with delegatesFocus = true
• Attach :focus styles
• Attach 'focus' event listener
• Disable element
• Focus element (mouse or JS)

Actual results:

• Element received styles
• document.activeElement changed to element
• focus event is never fired

Expected results:

• focus event should fire

Same should occur with keydown events. Chrome does fire keydown, focus, and blur. Chrome does not fire click events.

Click events on disabled elements aren't necessary for my use cases, but keydown, focus, and blur are. This is necessary to satisfy ARIA requirements of focusability of certain disabled elements (list items, menu items, etc).

https://w3c.github.io/aria-practices/#kbd_disabled_controls

If a disabled FACE can receive focus, it should fire a focus event and allow keyboard navigation.

If it a disable FACE cannot receive focus, it should not receive :focus state or be assigned to document.activeElement.

Codepen: https://codepen.io/shortfuse/pen/jOvqQGB

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Disability Access APIs' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

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

:edgar, since you are the author of the regressor, bug 1800370, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[clshortfuse]

There is a WPT related to a non-delegatesFocus element:

https://searchfox.org/mozilla-central/rev/edccfac5746704da49c1551aed8b79f57003bd68/testing/web-platform/tests/custom-elements/form-associated/form-disabled-callback.html#90-101

It checks everything I would think it should (activeElement and tabIndex), but it seems delegateFocus is borking it.

My current workaround is to detect Firefox and manually report and not focused:

/**
 * @param {Element} element
 * @return {boolean}
 */
export function isFocused(element) {
  if (!element) return false;
  if (IS_FIREFOX && element.constructor.formAssociated && element.hasAttribute('disabled')) {
    // https://bugzilla.mozilla.org/show_bug.cgi?id=1818287
    console.warn('Firefox bug 1818287: Disabled form associated custom element cannot receive focus.');
    return false;
  }
  if (document.activeElement === element) return true;
  if (!element.isConnected) return false;
  if (isInLightDOM(element)) return false; // extraneous 
  return element.matches(':focus');
}

It's an an acceptable workaround, but would much rather not have to skip items.

Comment 4

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

(In reply to clshortfuse from comment #3)

It checks everything I would think it should (activeElement and tabIndex), but it seems delegateFocus is borking it.

Yeah, a FACE Element is not a focusable area if it is disabled, but if it have a shadow root whose delegates focus is true, then it will delegate focus to the shadow tree. I think we probably should allow focus event on disabled element, the normal form control element, e.g. input, can not be a shadow host, so I think allowing focus event on disabled element should only affect FACE element.

And for keyboard event, we behave differently than other browsers on normal form control element, either, for example, if we have

<button disabled><span id=target tabindex=0>Span</span></button>
<script>
document.getElementById("target").focus();
</script>

and we focus the span element by calling focus() and then press keyboard, button stops propagate the keyboard event.
I think this is rare use case, so I am not aware there is bug filed for this. but for FACE+delegateFocus, this becomes noticeable, so we probably need to also allow keyboard event on disabled element.

Comment 5

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

Adam, do you have spare of time working on this?

• Add focus and keyboard event in to white list in https://searchfox.org/mozilla-central/rev/3ede9deb876ad5d6389cb51b371d4a4c8d788deb/dom/html/nsGenericHTMLElement.cpp#1939-1996, I suppose this should work. And it would be safer if we add prefs for them
• Add some test in https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/semantics/disabled-elements and maybe https://searchfox.org/mozilla-central/source/testing/web-platform/tests/custom-elements/form-associated.

If not, feel free to bounce it back to me.

Comment 6

[Adam Vandolder [:avandolder]]

I think I should be able to tackle this. For the tests, we want to make sure keyboard events propagate, and that delegatesFocus allows for focus events on disabled FACE right?

Comment 7

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

(In reply to Adam Vandolder [:avandolder] from comment #6)

For the tests, we want to make sure keyboard events propagate, and that delegatesFocus allows for focus events on disabled FACE right?

Exactly, thanks!

Comment 8

[Adam Vandolder [:avandolder]]

Attachment: Bug 1818287 - Disabled form-associated custom elements with delegateFocus should receive focus and keyboard events. r?edgar

Comment 9

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

Set release status flags based on info from the regressing bug 1800370

Comment 10

[Adam Vandolder [:avandolder]]

Followup bug for enabling this on all channels has been filed: https://bugzilla.mozilla.org/show_bug.cgi?id=1819938

Comment 11

[clshortfuse]

Sorry, I'm a little late, but I'm also noticing some funkiness with focusin and focusout.

Since Firefox doesn't support [inert] yet, I'm trying to workaround if a FACE element child (slotted) gets focus. The setup is (roughly):

<x-card disabled onfocusin="{ if (this.disabledState) tabOverSelf(); } ">
  <slot inert={disabledState}>
    <button>I should not get focus if tabbed</button>
  </slot>
</x-card>

So, I wanted to workaround with xCardElement.addEventListener('focusin') but it never gets fired. I'm trying on 112.0a1 (2023-03-02) (64-bit) nightly, but it it could be PEBKAC, since I'm not sure if I'm on the right nightly. If focusin firing happens as a consequence of focus, then it should be fine.

Comment 12

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to clshortfuse from comment #11)

Sorry, I'm a little late, but I'm also noticing some funkiness with focusin and focusout.

Since Firefox doesn't support [inert] yet, I'm trying to workaround if a FACE element child (slotted) gets focus. The setup is (roughly):

FWIW. inert support is on the way, see this intent-to-ship

<x-card disabled onfocusin="{ if (this.disabledState) tabOverSelf(); } ">
  <slot inert={disabledState}>
    <button>I should not get focus if tabbed</button>
  </slot>
</x-card>

So, I wanted to workaround with xCardElement.addEventListener('focusin') but it never gets fired. I'm trying on 112.0a1 (2023-03-02) (64-bit) nightly, but it it could be PEBKAC, since I'm not sure if I'm on the right nightly. If focusin firing happens as a consequence of focus, then it should be fine.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1848bf682441
Disabled form-associated custom elements with delegateFocus should receive focus and keyboard events. r=edgar

Comment 14

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

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

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1848bf682441

Comment 16

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

Upstream PR merged by moz-wptsync-bot

Comment 17

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

Set release status flags based on info from the regressing bug 1800370

Comment 18

[Adam Vandolder [:avandolder]]

Attachment: Bug 1818287 - Part 2: Add tests for the propagation of focusin and focusout events through a disabled form-associated custom element. r?edgar

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a4ec639e1547
Part 2: Add tests for the propagation of focusin and focusout events through a disabled form-associated custom element. r=edgar

Comment 20

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

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

Comment 21

[Sandor Molnar[:smolnar]]

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

Comment 22

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

Upstream PR merged by moz-wptsync-bot

Comment 23

[clshortfuse]

Thank you, guys. I can see it's working on Firefox nightly now :)