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

Fixing issue found in IE9+ with change event dispatch #42

Merged
merged 1 commit into from Nov 1, 2014
Merged

Fixing issue found in IE9+ with change event dispatch #42

merged 1 commit into from Nov 1, 2014

Conversation

dominicbarnes
Copy link
Contributor

In my app, I discovered that in IE9 the change event would not fire as expected. Basically,
only a handler attached via onchange = function () { ... }; would work, all others such as
those bound via addEventListener would not fire at all.

After closer investigation, the condition used to test whether to use dispatchEvent or fireEvent
was not catching IE9+ correctly. (ie: typeof Event === "object") so I've changed this conditional
to hopefully be simpler and more accurate.

In my app, I discovered that in IE9 the `change` event would not fire as expected. Basically,
only a handler attached via `onchange = function () { ... };` would work, all others such as
those bound via `addEventListener` would not fire at all.

After closer investigation, the condition used to test whether to use `dispatchEvent` or `fireEvent`
was not catching IE9+ correctly. (ie: `typeof Event === "object"`) so I've changed this conditional
to hopefully be simpler and more accurate.
@abpetkov
Copy link
Owner

abpetkov commented Nov 1, 2014

How does that change made in switchery.js gives us any value? You're correct IE9 has issues when trying to change the checkbox state via another element (e.g. button), but if you make this change on the change event you manually fire on button click, it would fix it. But it makes no difference to be in the main file. Here's a demo which doesn't work in IE9, but will work if using dispatchEvent in the if statement.

@dominicbarnes
Copy link
Contributor Author

I'm not sure I follow your objection.

I was using switchery in another project and IE9 was not working correctly. IE9 reports the typeof as "object", instead of "function". But the check doesn't look necessary anyways, since the goal was to make sure we chose properly between dispatchEvent and fireEvent.

This fixes IE9, and continues to work for other browsers, unless you specifically know of something I've broken here?

@abpetkov
Copy link
Owner

abpetkov commented Nov 1, 2014

I have no objection. I'm trying to understand what your problem was, since you haven't shared it and how does this solve it. I messed around with this and I don't see any difference with how it currently is. I understand it all, I just don't know the problem you came across. The only problem I found is the one I described above - button to change input state.

@dominicbarnes
Copy link
Contributor Author

Check out this demo, you'll need to open the console.

In most browsers, both of those change events are fired. In IE9, only the one assigned via elem.onchange = function () { ... }; is fired.

@dominicbarnes
Copy link
Contributor Author

In this demo, I've attached multiple handlers via elem.addEventListener("change", function () { ... });.

@abpetkov
Copy link
Owner

abpetkov commented Nov 1, 2014

Seems legit. Thanks!

abpetkov added a commit that referenced this pull request Nov 1, 2014
Fixing issue found in IE9+ with change event dispatch
@abpetkov abpetkov merged commit e3fab01 into abpetkov:master Nov 1, 2014
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.

None yet

2 participants