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

findCanvasEventTarget doesn't use specialHTMLTargets #22942

Open
JoeOsborn opened this issue Nov 15, 2024 · 2 comments · May be fixed by #22959
Open

findCanvasEventTarget doesn't use specialHTMLTargets #22942

JoeOsborn opened this issue Nov 15, 2024 · 2 comments · May be fixed by #22959

Comments

@JoeOsborn
Copy link
Contributor

Please include the following in your bug report:

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.71 (4171ae200b77a6c266b0e1ebb507d61d1ade3501)
clang version 20.0.0git (https:/github.com/llvm/llvm-project d6344c1cd0d099f8d99ee320f33fc9254dbe8288)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /home/jcoa2018/.local/src/emsdk/upstream/bin

This line in library_html5.js:

// If that is not found either, query via the regular DOM selector.

Seems like it should fall back to findEventTarget, not manual document querying/ID lookup. At a minimum, it should check specialHTMLTargets[target] as well as the other options.

Right now, if you follow the docs and call your canvas e.g. "!canvas" in specialHTMLTargets, createContext seemingly won't be able to find it.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 15, 2024

It looks like this only effects OFFSCREENCANVAS_SUPPORT right? Otherwise findCanvasEventTarget does map to findEventTarget.

@juj WDYT? I guess that fallback should simply be findEventTarget?

@JoeOsborn
Copy link
Contributor Author

I like this patch for the return statement of findCanvasEventTargets but an explicit fallback could make sense too:

    // First check out the list of OffscreenCanvases by CSS selector ID ('#myCanvasID')
    return GL.offscreenCanvases[target.substr(1)] // Remove '#' prefix
    // If not found, if one is querying by using DOM tag name selector 'canvas', grab the first
    // OffscreenCanvas that we can find.
    // If that is not found either, query via the regular DOM selector.
     || (target == 'canvas' && Object.keys(GL.offscreenCanvases) && GL.offscreenCanvases[Object.keys(GL.offscreenCanvases)[0]])
     || specialHTMLTargets[target]
#if PTHREADS
     || (typeof document != 'undefined' && document.querySelector(target));
#else
     || document.querySelector(target);
#endif

JoeOsborn added a commit to JoeOsborn/emscripten that referenced this issue Nov 19, 2024
…ndEventTarget, and fix default canvas lookup bug
@JoeOsborn JoeOsborn linked a pull request Nov 19, 2024 that will close this issue
JoeOsborn added a commit to JoeOsborn/emscripten that referenced this issue Nov 21, 2024
…ndEventTarget, and fix default canvas lookup bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants