Repository navigation
Don't throw when the Pointer Lock API is missing - #28103
Conversation
Since emscripten-core#24603, `glfwSetInputMode(..., GLFW_CURSOR_NORMAL)` calls `document.exitPointerLock()` unconditionally. In browsers without the Pointer Lock API (Safari on iOS) that throws, so GLFW apps that set the cursor mode fail to start. Before emscripten-core#24603 these calls went through no-op fallbacks set up in `Browser.init`. Make the pointer lock calls in libglfw.js, libsdl.js and libbrowser.js that emscripten-core#24603 left unguarded no-ops when the API is missing, and test GLFW with the API removed. Fixes emscripten-core#28102 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request safely handles environments that lack Pointer Lock API support (such as Safari on iOS) by using optional calls to requestPointerLock and exitPointerLock across Emscripten's browser, GLFW, and SDL libraries, alongside adding a new test case to verify this behavior. The review feedback points out that using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains. It is recommended to use standard conditional checks instead of optional chaining to ensure backward compatibility.
| if (getFullscreenElement() === canvasContainer) { | ||
| canvas.exitFullscreen = Browser.exitFullscreen; | ||
| if (Browser.lockPointer) canvas.requestPointerLock(); | ||
| if (Browser.lockPointer) canvas.requestPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (Browser.lockPointer && canvas.requestPointerLock) canvas.requestPointerLock();| win.inputModes[mode] = value; | ||
| canvas.removeEventListener('click', GLFW.onClickRequestPointerLock, true); | ||
| document.exitPointerLock(); | ||
| document.exitPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (document.exitPointerLock) document.exitPointerLock();| win.inputModes[mode] = value; | ||
| canvas.addEventListener('click', GLFW.onClickRequestPointerLock, true); | ||
| canvas.requestPointerLock(); | ||
| canvas.requestPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (canvas.requestPointerLock) canvas.requestPointerLock();| if (getFullscreenElement() === canvasContainer) { | ||
| canvas.exitFullscreen = Browser.exitFullscreen; | ||
| if (Browser.lockPointer) canvas.requestPointerLock(); | ||
| if (Browser.lockPointer) canvas.requestPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (Browser.lockPointer && canvas.requestPointerLock) canvas.requestPointerLock();| case 0: // SDL_DISABLE | ||
| if (Browser.isFullscreen) { // only try to lock the pointer when in full screen mode | ||
| Browser.getCanvas().requestPointerLock(); | ||
| Browser.getCanvas().requestPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (Browser.getCanvas().requestPointerLock) Browser.getCanvas().requestPointerLock();| return 1; | ||
| case 1: // SDL_ENABLE | ||
| document.exitPointerLock(); | ||
| document.exitPointerLock?.(); |
There was a problem hiding this comment.
Using ES2020 optional chaining (?.) in Emscripten system libraries can cause compatibility issues with older JavaScript environments or build toolchains that do not downlevel modern JS syntax. It is safer and more backward-compatible to use standard conditional checks.
if (document.exitPointerLock) document.exitPointerLock();
|
Thanks for the review. I kept
Generated with Claude Code |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for fixing this. Sorry I didn't realize when i landed #24603 that not all browsers supported this API. |
Since #24603,
glfwSetInputMode(..., GLFW_CURSOR_NORMAL)callsdocument.exitPointerLock()unconditionally. In browsers without the Pointer Lock API (Safari on iOS) that throws, so GLFW apps that set the cursor mode fail to start. Before #24603 these calls went through no-op fallbacks set up inBrowser.init.Make the pointer lock calls in libglfw.js, libsdl.js and libbrowser.js that #24603 left unguarded no-ops when the API is missing, and test GLFW with the API removed.
Fixes #28102
Tested by building the updated
test_glfw3.cwith emsdk 4.0.19 and the new test's pre-js, then running it in Chrome: it throwsTypeError: document.exitPointerLock is not a functionwithout this change and exits with 0 with it. I could not runtest_browser.test_glfw3_no_pointer_lockitself locally.Generated with Claude Code