Repository navigation
Conversation
Previously `GLFW.onKeydown` unconditionally called `event.preventDefault()` on `Backspace` and `Tab` keydown events on `window`, preventing users from deleting text or tabbing between HTML `<input>` and `<textarea>` elements on the same page. Based on emscripten-core#22879 by @uysalibov and tests from emscripten-core#23285 by @kripken. Fixes: emscripten-core#22866
There was a problem hiding this comment.
Code Review
This pull request modifies libglfw.js to prevent default backspace and tab behaviors only when the event target is not an INPUT or TEXTAREA element, and adds a browser test to verify this functionality. The review feedback points out two important issues: first, simulateKeyEvent is undefined in the test file, which will lead to a runtime error; second, the event handler should guard against a null event.target and also avoid preventing default behaviors on contenteditable elements.
| EM_ASM({ | ||
| // Create input and textarea elements and add them to the DOM. |
There was a problem hiding this comment.
The test uses simulateKeyEvent to dispatch events and assert whether they were prevented, but simulateKeyEvent is not defined anywhere in this file or in the global scope. This will cause a ReferenceError when the test runs. We should define simulateKeyEvent inside the EM_ASM block.
EM_ASM({
const simulateKeyEvent = (type, keyCode, code, key, target) => {
target = target || Module['canvas'];
const event = new KeyboardEvent(type, {
keyCode: keyCode,
code: code,
key: key,
bubbles: true,
cancelable: true
});
Object.defineProperty(event, 'keyCode', { get: () => keyCode });
return target.dispatchEvent(event);
};
// Create input and textarea elements and add them to the DOM.| if (event.target.tagName !== 'INPUT' && event.target.tagName !== 'TEXTAREA' && | ||
| (event.key == 'Backspace' || event.key == 'Tab')) { |
There was a problem hiding this comment.
To prevent potential runtime errors, we should guard against event.target being null or undefined before accessing its properties. Additionally, we should also avoid preventing default behavior for contenteditable elements (e.g., rich text editors) on the same page, as users would expect Backspace and Tab to work there as well. Checking event.target.isContentEditable handles this case cleanly.
if (event.target &&
event.target.tagName !== 'INPUT' &&
event.target.tagName !== 'TEXTAREA' &&
!event.target.isContentEditable &&
(event.key == 'Backspace' || event.key == 'Tab')) {
Previously
GLFW.onKeydownunconditionally calledevent.preventDefault()onBackspaceandTabkeydown events onwindow, preventing users from deleting text or tabbing between HTML<input>and<textarea>elements on the same page.Based on #22879 by @uysalibov and tests from #23285 by @kripken.
Fixes: #22866