web: prevent_default depending on keys

[Vrixyz]

I'm contributing on bevy_egui, comparing its functionalities with eframe.

eframe has a fine control over which keys it wants to prevent its defaults or not, I think this could be a good addition to winit:
https://github.com/emilk/egui/blob/307565efa55158cfa6b82d2e8fdc4c4914b954ed/crates/eframe/src/web/events.rs#L101

This fine control allows eframe to support copy/paste/etc... while disallowing tabbing outside, going to previous page, and others.

[Vrixyz]

I suggest changing the current exposed prevent_default: bool to something like enum{Always, Check(Fn<KeyEvent, bool>), Never} ; not really sure of the impacts though.

[daxpedda]

Sounds reasonable to me.
I was trying to figure out if we can skip preventDefault() completely and rely on fine-grained events, unfortunately that's not possible, there isn't an event for every command: e.g. "F12", opening the developer console.

PR's welcome.

[daxpedda]

I think one thing we could do for sure is to detect copy/paste/cut shortcuts and not block those by default. I believe that's the main use-case currently for something like this anyway.

[ddfisher]

I'm looking at implementing this, but wanted to check in first.

My current thinking is to take a closure instead of adding an enum (i.e. change prevent_default from bool to Fn(web_sys::Event) -> bool). It'd still be straightforward for folks who want to have it always be true or false (|_| true is still pretty concise) and IMO adds a bit less complexity overall.

A couple drawbacks:

• This will have to be a (relatively minor) breaking change, as we wouldn't be able to support the WindowExtWebSys::prevent_default bool getter. I don't see any way around at least modifying the return value of this function if we want to support more fine-grained control here, though. My inclination is to just remove it, if that seems reasonable?
• There are a variety of event types which could call this closure, so users will need to use wasm_bindgen::dyn_ref (or other members of JsCast) to act on event types they care about. I'd love to make this more ergonomic, but I don't see a great way of doing that.

How does this sound as a way forward?

[daxpedda]

As pointed out in #2831 (comment):

I think one thing we could do for sure is to detect copy/paste/cut shortcuts and not block those by default. I believe that's the main use-case currently for something like this anyway.

I'm not sure what other use-cases are out there, my first impression is that this would satisfy the majority.
Hence: whats your use-case 😊?

[ddfisher]

Not calling preventDefault on copy/cut/paste shortcuts is an important part of my use-case as well, but I'd also like to allow some other common browser shortcuts that would otherwise be interrupted (e.g. ctrl-L to focus the address bar, ctrl-R to refresh, etc). The ability to decide which shortcuts the app intercepts vs which is passes to the browser in a fine grained way is pretty important to me from a design perspective.

The other plug I'd make for the general approach is that it would be easy to add convenience methods for the more specific use cases (like supporting copy/cut/paste), but it would still allow folks to customize the specific behavior when they need to for their particular application.

[daxpedda]

Not calling preventDefault on copy/cut/paste shortcuts is an important part of my use-case as well, but I'd also like to allow some other common browser shortcuts that would otherwise be interrupted (e.g. ctrl-L to focus the address bar, ctrl-R to refresh, etc). The ability to decide which shortcuts the app intercepts vs which is passes to the browser in a fine grained way is pretty important to me from a design perspective.

Would you mind expanding on that? At that point why aren't you just using WindowBuilder::prevent_default(false)? What is it that you are trying to prevent otherwise?

The other plug I'd make for the general approach is that it would be easy to add convenience methods for the more specific use cases (like supporting copy/cut/paste), but it would still allow folks to customize the specific behavior when they need to for their particular application.

The thing is that users can still just use WindowBuilder::prevent_default(false) and do their own fine-grained Event.preventDefault() strategy on the user level, so the minimum obligation from Winit's side, getting out of the users way, is fulfilled here. So we have to make a design decision here in how far Winit wants to go, which largely depends on user feedback really.

Particularly, I don't believe we can really expose something useful here. E.g. you proposed exposing Event and letting users figure it out by casting the type, but at this point a call to HTMLCanvasElement.addEventListener is not too far away anymore. Furthermore not all events have their own specialized type, so users might not always be able to figure out what event this is. E.g. keydown vs keyup.

Alternatively with #3432 we might let users just return a bool (just spit-balling) from each event to decide if they want to use Event.preventDefault() here.

Let me know what you think!

[ddfisher]

Would you mind expanding on that? At that point why aren't you just using WindowBuilder::prevent_default(false)? What is it that you are trying to prevent otherwise?

A couple examples: I want to act on things like instead of it tabbing through the browser UI, and I want ctrl-p to open a command palette instead of printing.

The thing is that users can still just use WindowBuilder::prevent_default(false) and do their own fine-grained Event.preventDefault() strategy on the user level, so the minimum obligation from Winit's side, getting out of the users way, is fulfilled here. So we have to make a design decision here in how far Winit wants to go, which largely depends on user feedback really.

Particularly, I don't believe we can really expose something useful here. E.g. you proposed exposing Event and letting users figure it out by casting the type, but at this point a call to HTMLCanvasElement.addEventListener is not too far away anymore. Furthermore not all events have their own specialized type, so users might not always be able to figure out what event this is. E.g. keydown vs keyup.

Ahh, I think you're right -- adding an event listener to the HTMLCanvasElement seems like a great way to go here. Sorry -- I completely forgot about that possibility! I wonder if adding some docs to WindowBuilderExtWebSys recommending that to users who need more fine-grained control would be worthwhile? I can write something up if you think that'd be worth having.

[ddfisher]

Alternatively with #3432 we might let users just return a bool (just spit-balling) from each event to decide if they want to use Event.preventDefault() here.

This would definitely be handy!

[daxpedda]

I wonder if adding some docs to WindowBuilderExtWebSys recommending that to users who need more fine-grained control would be worthwhile? I can write something up if you think that'd be worth having.

Absolutely, adding a little guide to WindowBuilderExtWebSys::with_prevent_default() would be great! Off the top of my head:

• Mention which events Winit usually takes care of.
• How and which events users may want to deal with themselves.
• Maybe a little code example? But that might be too much.

[mcclure]

I entered a feature request #3797, it got closed as a dupe of this one, but with the note that this bug saw little movement in part due to the perceived limited range of use cases. I have another possible use case to add.

Here's a winit+webgpu app I made (because webgpu is an emerging tech, this might only work on Chrome):

https://data.runhello.com/j/wgpu/special/winit-swallow

and an alternate version, where I call window.set_prevent_default(false); at the start of the program:

https://data.runhello.com/j/wgpu/special/winit-swallow-2

What I want to call attention to is that in the default implementation right-clicking on the entire window is suppressed, whereas in the second implementation it works. Not having access to right-click in the first one means it's a good bit harder to get into the Chrome inspector (where for example I might look at my app's error messages), it also completely excludes use of the "copy image"/"save image as…" verbs Chrome offers for canvases. (In currently shipping Chrome these menu items are grayed out for WebGPU canvases, but will still work for WebGL and Canvas2D.) In Firefox you can shift-right-click and this is always handled by the browser and not your app, but Chrome doesn't seem to have this feature.

I can get the right-clicks back with set_prevent_default(false), but the thing is I usually don't want the default behavior, I only want to pass through right clicks. For example I might want to add app-specific handling for clickdrag or scrollwheel, and any default behaviors simultaneously occurring for those would be pretty bad. (In the current app in Chrome on Windows it appears scrollwheel is suppressed by my CSS, but I'm not sure whether scrollwheel would be so easily suppressed in for example Safari on Mac, and anyway the CSS is an application detail; you could easily imagine a scrollable window with a Winit-driven WebGPU canvas that scrolls with the window, and has a need to suppress browser scrollwheel when the cursor is over the canvas but doesn't want to suppress browser whiteclick.

I've taken a look at @ddfisher's docs and I'm sorry but I'm not sure if I understand them or not :( It looks like I'd have to add a lot of boilerplate to my app, enough so that (for a simple graphics effect like my current app) a significant percentage of my total code would be just doing event handling to express the single idea of "when running on Web, swallow events that aren't a right click". My preferred API would be if we could keep set_prevent_default but then during an event there could be a prevent_default_current(override_value:bool) on either Event<T> or ActiveEventLoop so that special handling would only need to be added in those functions that are the exception to the currently-set overall flag. (A side-effecty approach like this may not be very Rusty, but I'd prefer to be able to interleave it with my regular event handler code so that I can add smart discrimination. For example I don't want to passthrough all mouse events, only ones that specifically turn out to correspond to a right-click. Or maybe my app has a right-click menu, and I so want to do what Google Drive does and swallow right-clicks while passing through shift-right-clicks.)

[daxpedda]

I think #3451 clearly shows that this actually doesn't take a lot of effort. Its currently something around 60 lines and I'm pretty sure we can whittle it down to half that.

Opening developer tools I believe is a weak use case because this can be easily circumvented if you know how. I test Winit on Chrome regularly by de-focusing the canvas by clicking on the address bar and then press F11.

That said, I have to clarify that in my perspective we have already long cleared the threshold of usefulness here. The problem is just how to design an API around the current limitations of Winit.

prevent_default_current(override_value:bool) is actually the best idea I've heard so far so I'm happy to move ahead with that until the new API re-design. This API could potentially be also safe to backport to v0.30.

I would be happy to review a PR implementing this, otherwise I will get to it myself at some point, but its probably on the bottom of my priority list for Winit right now.