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.onchange` is never called #9575

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

`input.onchange` is never called #9575

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

Comments

@Gozala
Copy link
Contributor

@Gozala Gozala commented Feb 8, 2016

I believe change events are never dispatched. Here is simple example

<html>
<body>
<input id='input' autofocus="true" 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.target.value}\n`

body.onchange = onEvent
input.onchange = onEvent
</script>
</html>

Type s in input & click blanch space. You should see following:

INPUT : change - s
BODY : change - s

in servo you'll see nothing.
@Gozala
Copy link
Contributor Author

@Gozala Gozala commented Feb 8, 2016

/cc @paulrouget
@jdm
Copy link
Member

@jdm jdm commented Feb 8, 2016

The checkbox and radio code dispatches a "change" event, but the oddly-named ChangeEventRunnable in htmlinputelement.rs only dispatches an "input" event.
@jdm jdm added the A-content/dom label Feb 8, 2016
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 14, 2016

To support this we may need to add a focus/blur virtual method. onchange is dispatched (for text inputs) when the element is blurred and the value has changed since the last focus, so we need HTMLInputElement to keep track of that.

I don't particularly like adding virtual methods, though.

(Aside: This is another one of those events which gets messed up totally if you change the input element's type in mid-air)
@agi90 agi90 mentioned this issue Jul 17, 2016
Closed
3 of 5 tasks complete
@agi90
Copy link

@agi90 agi90 commented Jul 17, 2016

I added a pull request creating a virtual method as mentioned by Manishearth. Please let me know if you'd rather follow a different approach.
bors-servo added a commit that referenced this issue Jul 17, 2016
@bors-servo
Auto merge of #12475 - agi90:html-input-change-event, r=<try>
Loading status checks…
34ae2f3 
Dispatch change event for HTMLInputElement.

I took a stab at #9575. This is my first pull request on servo so please bear with me :-).

I added a new virtual function `before_handle_event` which is a little more generic than needed but it looks like it will be needed for other things? At least judging by Gecko's codebase. Please let me know if you'd rather have a method just for blur.

I'm not quite sure why I couldn't use atom!("blur") instead of Atom::from("blur").

If this is the right direction I can write tests for this change (probably importing them if there aren't any already).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #9575 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12475)
<!-- Reviewable:end -->
@jdm jdm mentioned this issue Jan 29, 2018
Closed
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.

Dispatch change event for HTMLInputElement.
6 participants
@Gozala @jdm @paulrouget @Manishearth @agi90 @cbrewster
You can’t perform that action at this time.