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

Prevent browser forward/backward navigation keys from crashing editor #1631

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skoriop
Copy link
Contributor

@skoriop skoriop commented Feb 28, 2024

This adds the BrowserBack and BrowserForward browser control keys to ModifierKeys.

Closes #1628.

@Keavon
Copy link
Member

Keavon commented Feb 28, 2024

I'm thinking that the Rust backend never has a use for browser state, since that's a concept that doesn't apply outside the browser context (like when we have a desktop app through Tauri). Since it won't be used, I'd prefer taking the approach of blocking such events from ever being sent to the Rust backend while it's still in the JS context, as it should remain a purely JS concern. Could you please try updating your approach to do that and test that it still indeed fixes the crash? Thanks!

@Keavon Keavon changed the title Add browser control keys to ModifierKeys Prevent browser forward/backward navigation keys from crashing editor Feb 28, 2024
@Keavon
Copy link
Member

Keavon commented Feb 28, 2024

I should also mention that I wasn't able to reproduce this crash when I just tested it. (See my comment from just now in the issue.) Were you able to reproduce it?

@skoriop
Copy link
Contributor Author

skoriop commented Feb 28, 2024

I should also mention that I wasn't able to reproduce this crash when I just tested it. (See my comment from just now in the issue.) Were you able to reproduce it?

No, not really, I thought it was an issue with my mouse

@0HyperCube
Copy link
Member

I disagree that we should discard the forwards and backwards buttons as you previously mentioned that they should be used for undoing changes in selection @Keavon.

I can trivially reproduce on chrome by creating a new document and then pressing the forwards button.

@Keavon
Copy link
Member

Keavon commented Feb 28, 2024

@0HyperCube wait, are we talking about mouse buttons or the clickable buttons left off the URL bar in the browser? I was interpreting this to be the latter.

@Keavon
Copy link
Member

Keavon commented Feb 28, 2024

Ok, I misunderstood, now I get what's going on with it being a mouse button. This will also be part of #706.

Disregard my earlier statement about blocking it in the JS, sorry for the confusion. However I don't think it's accurate for these to be considered modifier keys. In the JS, the necessary code should be used to interpret it (in all browsers, since apparently this particular one is specific to Chrome?) and send it as a new NavigateBack and NavigateForward key which can be added to the Key enum in input_keyboard.rs.

@0HyperCube
Copy link
Member

I'm not sure you understand that I'm referring to mouse buttons (i.e. buttons on the mouse). These are not keyboard keys and thus should not be classified as such. As per the aforementioned link to https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons#value we can see that the mouse buttons are standardised and therefore it would not make sense to overcomplicate this issue by trying to re-invent keyboard input systems.

@Keavon
Copy link
Member

Keavon commented Feb 28, 2024

We currently extend the W3C keyboard button spec with the mouse buttons (here) in our backend input handling code, which lets us treat a button press on the mouse the same way we'd treat a button press on the keyboard. We'll also be able to use buttons on a stylus, a 3D mouse, a joystick or gamepad, a VR controller, or any other exotic input device the same way (some of these could be used to drive real time animation in the future, perhaps). In other words, our backend (at least based on its current architecture) doesn't care what physical device the buttons are part of, we just care that it's a physical button that can be pressed and released (as opposed to something fundamentally different like an axis input, a scroll wheel, fingers on a touchscreen, a gesture, etc.).

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.

Crash when using mouse's forward/backward navigation keys
3 participants