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

bind addEventListener, removeEventListener, and dispatchEvent for browsers that support EventTarget #106

Merged
merged 1 commit into from Apr 13, 2018

Conversation

cvazac
Copy link
Contributor

@cvazac cvazac commented Mar 20, 2018

In supporting browsers (Chrome, Safari, Edge, Firefox), third-party script loaded in an IFRAME that wraps EventTarget.prototype.addEventListener will lose track of the context of this, and will incorrectly bind the handler to its own IFRAME (instead of top).

Reproducible Case

To reproduce the issue, go here with the console open.
Without the fix, you will only see only 2 handlers fire:

first load handler fired
second load handler fired

To see the fix in action, go here.
You will see:

first load handler fired
second load handler fired
third load handler fired

Why this happens

in history.js:

window.addEventListener = addEventListener

in an IFRAME:

top.EventTarget.prototype.addEventListener = (function(_addEventListener){
  return function(){
    return _addEventListener.apply(this, arguments)
  }
})(top.EventTarget.prototype.addEventListener)

developer code in top:

window.addEventListener('load', callback)

Because history.js basically unbinds the window.addEventListener method, this passed to the _addEventListener.apply call will be the inner IFRAME (should be === top), and the load listener will be bound to the wrong window.

function maybeBindToWindow(func) {
if (window &&
window.EventTarget &&
typeof window.EventTarget.prototype.addEventListener === 'function' &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work in IE8?

I also need to support IE8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.EventTarget doesn't appear in a microsoft browser until edge (not even IE11!)

@devote devote merged commit 4920ca4 into devote:master Apr 13, 2018
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