Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot press escape key to close modal dialog #12811

Open
gabalafou opened this issue Aug 22, 2024 · 1 comment · May be fixed by #12813
Open

Cannot press escape key to close modal dialog #12811

gabalafou opened this issue Aug 22, 2024 · 1 comment · May be fixed by #12813
Labels
html search javascript Pull requests that update Javascript code type:bug

Comments

@gabalafou
Copy link

gabalafou commented Aug 22, 2024

Describe the bug

If you add an HTML modal dialog to a website built with Sphinx, and you have search enabled, then when the modal is shown, the escape key is broken, meaning you should be able to close the modal dialog by pressing the escape key, but you can't

This is because sphinx_highlight.js calls event.preventDefault() on line 142 when the escape key is pressed.

Steps to verify this explanation. Follow the steps to reproduce (below). Then open Chrome Dev Tools. Go to the Sources tab, expand Event Listener Breakpoints, expand Keyboard, and check the keydown checkbox. Then click on the modal dialog and press Escape. Use the debugger to step through the code until you get to the event.preventDefault() on line 142 of sphinx_highlight.js.

The easy fix would be to add "DIALOG" to the BLACKLISTED_KEY_CONTROL_ELEMENTS set. However, I think the appearance of this bug suggests that the current implementation could be made more robust.

I'm not quite sure I totally understand the logic of not handling the escape key when focus is on a textarea, input, select, or button, which is the current set of elements covered by BLACKLISTED_KEY_CONTROL_ELEMENTS. I get that, with a select, the escape key is meant to close it. But with a button, is there any expected default behavior for the escape key? In other words, if buttons are excluded, why not also links or any other kind of focusable element?

Which makes me wonder if a safer and just as effective solution would be to bail if document.activeElement is anything other than document.body or null.

How to Reproduce

Add a native HTML modal dialog to a Sphinx site. You do this by adding a <dialog> tag and opening it with .showModal(). For example, you could extend layout.html, by creating the following file at docs/source/_templates/layout.html:

{% extends "!layout.html" %}
{% block document %}
    <dialog id="dialog">Hello, world</dialog>
    <script defer>
        const dialog = document.getElementById("dialog");
        dialog.showModal();
    </script>
    {{ super() }}
{% endblock %}

Steps:

  1. Load the index page, the modal dialog should open automatically, centered over the page.
  2. Press the escape key.
  3. Observe that nothing happens, even though it should close the modal dialog.

Environment Information

Platform:              darwin; (macOS-14.6.1-arm64-arm-64bit)
Python version:        3.10.10 | packaged by conda-forge | (main, Mar 24 2023, 20:12:31) [Clang 14.0.6 ])
Python implementation: CPython
Sphinx version:        7.2.6
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.17.2

Sphinx extensions

extensions = []

Additional context

This bug surfaced while working on a pull request in another GitHub repo (pydata/pydata-sphinx-theme#1942). To get around this bug, we had to manually reconnect the escape key to the modal dialog.

@jayaddison
Copy link
Contributor

I'm not quite sure I totally understand the logic of not handling the escape key when focus is on a textarea, input, select, or button, which is the current set of elements covered by BLACKLISTED_KEY_CONTROL_ELEMENTS. I get that, with a select, the escape key is meant to close it. But with a button, is there any expected default behavior for the escape key? In other words, if buttons are excluded, why not also links or any other kind of focusable element?

This seems likely to be accessibility-related: a mechanism to reduce conflict with input assistance technologies that might use the escape key as a modifier or menu control. That said, I don't know for certain that that is why it was implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
html search javascript Pull requests that update Javascript code type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants