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

[WIP] Do not preventDefault() for Ctrl+v paste event #19510

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Karm
Copy link

@Karm Karm commented Jun 2, 2023

Hello Emscripten,

There has been somewhat lengthy debate on how to handle Copy/Paste events in our imgui WASM apps cleanly. According to my search, this thread on HelloImgui project repo is a good summary.

@slowriot came up with a nice single header that apparently solves the integration just about right.

It didn't work for me though, I had to use mouse middle button to generate the paste event. An experience I summarized over here in this post.

Following @pthom's suggestions, I realized that the change outlined in this PR helps me.
My ImGui SDL backed WASM app receives paste events now and I have a seamless Copy/Paste experience on my Linux Firefox system.

I do not think this is supposed to be the final solution as it is definitely missing some corner cases I am oblivious to.

if ({{{ makeDynCall('iiii', 'callbackfunc') }}}(eventTypeId, keyEventData, userData)) e.preventDefault();
if ({{{ makeDynCall('iiii', 'callbackfunc') }}}(eventTypeId, keyEventData, userData)) {
if(!(event.ctrlKey && event.key == 'v')) {
e.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to special-case control-V here... I can see the motivation.

I worry that in general we don't have a principled approach to this in our JS libraries - I think SDL, glfw etc. each do their own thing. Which is not great.

It would be best to think about a general solution here. I'm not sure what that would be. But something that works for them all would be good. In theory, if we could have an API emscripten_ignore_paste_commands() that opts into this behavior, and does that for HTML5, SDL, glfw, etc., that seems best in theory, but I'm not sure if that's practical to do.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @kripken for the comment. I'd assess my position here with this meme.

So emscripten_ignore_paste_commands() would touch library_html5.js, library_sdl.js, library_glfw.js, library_html5_webgl.js and one could call it regardless of the "backend"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be a good argument here for an opt-in to this behaviour, or some other kind of switch.

@Karm, thanks for testing my little paste library - I'm just catching up with all the discussion, but it looks like the issue you've hit is SDL-specific. I tested exclusively on GLFW, and had ctrl-V working consistently in that environment.

I would want to check that this proposed change doesn't break the existing behaviour for GLFW (as it already works as-is). The easiest way to achieve that may indeed be just to have the behaviour made switchable.

Copy link
Contributor

@slowriot slowriot Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put together a simple demo to show emscripten-browser-clipboard working with GLFW: https://armchair-software.github.io/emscripten-clipboard-demo/

Source: https://github.com/Armchair-Software/emscripten-clipboard-demo

For me, copy, cut, and paste work with ctrl-c, ctrl-x and ctrl-v in Firefox and Chromium on the above demo. The emscripten version is out of the box, no modifications needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slowriot I noticed that while Sokol based app, e.g. https://pthom.github.io/imgui_manual_online/manual_sokol/imgui_manual.html?version=1 works across the board even on tapping the screen in Firefox on my Android, your example (and my app) does not respond to such a tap attempt and thus doesn't let me paste on phone.

So the events we are accidentally swallowing on SDL (and GLFW?) are also touch (tap?) and cmd on mac?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on SDL, but I don't believe GLFW is accidentally swallowing anything. In my test application, I made no attempt to collect or process touch events. To do so would require use of the Emscripten touch api and then passing those inputs as mouse inputs to GLFW and hence to ImGui. It could possibly benefit from some adaptation of GLFW's multitouch feature as well. Whereas with SDL, I believe all that comes out of the box, so there's no direct equivalence. I think touch events are a red herring here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I don't have any meaningful analysis at hand. I merely gave it a try on my cellphone. Let's leave touch events out of this particular issue 👍

WerWolv added a commit to WerWolv/ImHex that referenced this pull request Feb 13, 2024
…#1542)

### Problem description
WASM build does not support copy/paste beyond the application. Meaning,
there's no practical way of sending text back and forth across the
application border.

There are lengthy threads why this is a technical challenge in
WASM/Browser world, e.g:
- pthom/hello_imgui#3
- emscripten-core/emscripten#19510

### Implementation description
Implements a workaround solution as Header only C++ library, as proposed
and implemented at:
https://github.com/Armchair-Software/emscripten-browser-clipboard

Maybe there are cleaner ways of achieving the functionality. Definitely
would like to have some discussion around this. 👀

ℹ️ The proposed PR "works for me" on Windows, using CTRL-C/V shortcuts
to copy text from and to the application. On MacOS the system shortcut
to Paste is different from what ImHex has defined. This results in
system Paste shortcut of command-V triggering the browser callback to
synchronise the application clipboard, but no actual Paste takes place
within ImHex.

If there would be a clean way to trigger the paste command, that would
be wonderful (or get the context and references to write the data to the
cursor, but I was unable to find a clean solution). The only proposed
solutions in the referenced threads were about triggering paste event
internally via Key events. This seemed wonky 🙃 , so is not currently
implemented. At the moment the paste on MacOS is command+V followed by
control+V.

### Additional things
This is definitely a stopgap solution before the ImGui and Emscripten
take a more proper solution in enabling Copy/Paste outside the
application borders. However, I feel like this is a must have capability
to make the WASM build more useful, not just for trying out ImHex.

Cheers! 🍻

---------

Co-authored-by: Nik <[email protected]>
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 this pull request may close these issues.

3 participants