-
Notifications
You must be signed in to change notification settings - Fork 46
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
Ensure target and currentTarget properties are set on events #34
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change requires corresponding tests
Thank you. I added some tests, please let me know if they're not sufficient. Also, running the tests updated the built |
I'm picking this issue up, so I can move our company off our forks of FakeXMLHttpRequest, pretender, and mirage. If we can get this issue fixed up, and bump all the versions so that the fix is available to mirage, then we won't need our forks. @stefanpenner If you get a moment, are you able to review this PR? I can make any necessary changes from the review. |
Looking at Comparison of Event Targets on MDN
I think this may even be related to #28? |
When listening for events (e.g. readystatechange) some libraries expect `event.currentTarget` to be set to the XHR, as well as `event.target`. Since the `readystatechange` event doesn't bubble, the `target` and `currentTarget` will always be the same.
Set the event target for the progress event via the constructor, for consistency with other events. Also ensures currentTarget is set at the same time.
I've rebased this PR onto the current master and compacted the commits down into two: one for |
@stefanpenner Is there anything else preventing this PR from getting accepted? This gets FakeXMLHttpRequest to behave more realistically, in addition to unblocking those of us who are relying on forks. |
We had a problem where a library was making XHRs and expecting
event.currentTarget
to be set to the XHR object in areadystatechange
handler.Set currentTarget on Events
When listening for events (e.g.
readystatechange
) some libraries expectevent.currentTarget
to be set to the XHR, as well asevent.target
. Since thereadystatechange
event doesn't bubble, thetarget
andcurrentTarget
will always be the same.Set the target for
readystatechange
eventsEnsure we can access the XHR via target/currentTarget when listening to
the
readystatechange
event.