Skip to content

/ servo

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

Jump to bottom

`input.onkeypress` is never called #9573

Open
Gozala opened this issue Feb 8, 2016 · 7 comments
Open

`input.onkeypress` is never called #9573

Gozala opened this issue Feb 8, 2016 · 7 comments
Labels
A-content/dom

Comments

@Gozala
Copy link
Contributor

@Gozala Gozala commented Feb 8, 2016

I run into this issue in browserhtml/browserhtml#863 it turns out servo never dispatches keypress events. Here is also a simple test case I've created:

<html>
<body>
<input id='input' type=text />
<pre id='output'></pre>
</body>
<script>
const input = document.querySelector('#input')
const output = document.querySelector('#output')
const body = document.body

const onEvent =
  event =>
  output.textContent += `${event.currentTarget.tagName} : ${event.type} - ${event.key}\n`

body.onkeydown = onEvent
body.onkeypress = onEvent
body.onkeyup = onEvent
body.onchange = onEvent
</script>
</html>

If you type into input field and type s you should see:

BODY : keydown - s
BODY : keypress - s
BODY : keyup - s

in servo you'll see following instead:

BODY : keydown - s
BODY : keyup - s
@Gozala
Copy link
Contributor Author

@Gozala Gozala commented Feb 8, 2016

Cc @paulrouget
@jdm jdm added the A-content/dom label Feb 8, 2016
@jdm
Copy link
Member

@jdm jdm commented Feb 8, 2016

HTMLInputEvent::handle_event calls PreventDefault when a keydown event causes the text input to change. Document::dispatch_key_event checks whether the keydown event was prevented before dispatching keypress. We need to resolve this mismatch.
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 14, 2016

I'm not sure why we need to prevent the default there? Or is that to stop it from bubbling further?

b0552cb added this.
@jdm
Copy link
Member

@jdm jdm commented Feb 14, 2016

It's to properly handle things like

<body onkeydown="...">
<input>

from executing the JS handler when the user modifies the input.
@jdm
Copy link
Member

@jdm jdm commented Feb 14, 2016

Which apparently is a totally misguided goal, given how Firefox treats that (ie. the JS handler is executed).
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 14, 2016

So, we should just remove it? (E-easy, perhaps)

I can understand setting Bubbles to false or doing something like that. DefaultPrevented isn't something our code should be messing with, and if we need a similar mechanism we should implement an internal one. But it doesn't look like one is necessary in this case.

Aside: There's a good chance above behavior with the JS handler is not consistent across browsers; I recall previous inconsistencies arising from nested form widgets and stuff (I can't recall exactly what), and this is pretty similar. The spec isn't very clear when it comes to these things, which is fine -- stop nesting inputable things, that's silly! -- and as far as keypress I can't find anything that tells us to do anything wrt the default or bubbling.
@jdm
Copy link
Member

@jdm jdm commented Feb 14, 2016

Nice, Firefox executes the onkeydown in the body element, and blink doesn't. I wonder what edge does?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@Gozala @jdm @paulrouget @Manishearth
You can’t perform that action at this time.