Closed Bug 1726454 Opened 3 years ago Closed 2 years ago

Spacebar performs two actions when summary element has focus

Categories

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

Firefox 92
defect

Tracking

()

VERIFIED FIXED
107 Branch
Tracking Status
firefox107 --- verified

People

(Reporter: dan, Assigned: emilio)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Give focus to a <summary> element inside a <details> element. Press the spacebar key to activate the open/close of the <summary>. This can be demonstrated inside the HTML Demo: <summary> example on MDN: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary

Actual results:

The disclosure summary expanded, but also activated the feature to scroll down a page, taking the disclosure out of the viewfinder.

Expected results:

While a disclosure has focus, activating the element should not scroll it out of the viewfinder. This presents a usability concern, and may confuse keyboard-only users. I would expect this feature to work how checkboxes do when using keyboard navigation.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: UI Events & Focus Handling' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → DOM: UI Events & Focus Handling
Product: Firefox → Core

I can't see the page being scrolled down when testing Nightly (93) but I can see that in 91.

Bug 1481400 was fixed already in 91, but were there some followups...

dan, could you try Nightly to see if the issue reproduces there for you, thanks.

Severity: -- → S3
Flags: needinfo?(dan)
Priority: -- → P3

Hi Olli,

I was able to replicate in Nightly 94.0a1 (2021-09-08) (64-bit).

I've recorded a video of the replication. At the beginning of the video I give the disclosure focus and press the enter key 4 times to open/close the details. Following those rapid succession open/close, I give the element focus again and use the spacebar to engage it. As you will see, the screen scrolls down, but when I return to the disclosure it has been activated.

https://cln.sh/jtC7eH

Selecting a disclosure that is lower on the page (rather than the demo at the top) yields a similar behavior, but due to CSS it is far more subtle. I didn't record my screen on that attempt, but when I do the same user interaction, the disclosure is opened and scrolled to the bottom of the content inside the details.

Flags: needinfo?(dan)

I can replicate on FF 100 / macOS. Attaching a reduced test case. Make sure you have a generous scroll area (you can increase the font size further), then simply Tab to the <summary> element, then press the Space key.

In addition to expanding the <details> element, the page gets scrolled.

I'm still seeing the bug on Firefox 104.0.2 (Windows 7 64-bit).

So the way I thought this was supposed to work was this:

However mKeyCode is 0, which doesn't make much sense and might be a DOM bug? The reason this works on other elements is:

So maybe we should remove the HandleKeyboardActivation preventdefault code and do that instead?

Masayuki, is it expected that mKeyCode is zero for the key press event?

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(masayuki)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

Masayuki, is it expected that mKeyCode is zero for the key press event?

Yes, in web content, we set same value to keyCode and charCode after bug 1502795. However, our chrome code may depend on the traditional behavior. Therefore, I considered to keep the traditional behavior in chrome script.

And note that the Space key is handled in a spacial path. If WidgetKeyboardEvent::mKeyNameIndex equals KEY_NAME_INDEX_USE_STRING and WidgetKeyboardEvent::mCodeNameIndex equals CODE_NAME_INDEX_Space (i.e., the space key is not mapped to a function key), it's treated as Space key for the page scroll for supporting any keyboard layouts.

And yes, Space key etc shouldn't be handled in editable content. E.g., data:text/html,<details contenteditable><summary>foo</summary>bar</details>.

Flags: needinfo?(masayuki)

Perhaps, the following method of WidgetKeyboardEvent should work for internal C++ event handlers:

[[nodiscard]] bool ShouldWorkAsSpaceKey() const {
  return
    // If the keyCode value is "Space", it should work as space key.
    mKeyCode == NS_VK_SPACE ||
    // Additionally, if the code value is "Space" and the key is not mapped to
    // a function key (i.e., not a printable key), we should treat it as space key
    // because the active keyboard layout may input different character from
    // the ASCII white space (U+0020).  For example, NBSP (U+00A0).
    (mKeyNameIndex == KEY_NAME_INDEX_USE_STRING &&
     mCodeNameIndex == CODE_NAME_INDEX_Space);
}
Flags: needinfo?(emilio)

This prevents <summary> from scrolling with the space bar. Add a test
for that.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6f819654ebc5 Make nsGenericHTMLElement::HandleKeyboardActivation handle spacebar key press correctly. r=masayuki
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36251 for changes under testing/web-platform/tests
Regressions: 1793606
Upstream PR was closed without merging
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d76885757ec1 Make nsGenericHTMLElement::HandleKeyboardActivation handle spacebar key press correctly. r=masayuki
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch
Upstream PR merged by moz-wptsync-bot

Reproduced the issue with Firefox 107.0a1 (2022-10-04) on Windows 10x64 using the attached test case from comment 4 and the Firefox View page. When pressing the Space key to close/expand the scroll action is also performed.
The issue is no longer reproducing with Firefox 107.0a1 (20221007093840) on Windows 10x64, macOS 11 and Ubuntu 20.04. The scroll action is no longer performed on the attached test case and on Firefox View page.

Status: RESOLVED → VERIFIED
Regressions: 1822860
Regressions: 1824686
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: