fix(Observable/Ajax): mount properties to origin readystatechange fn

[Brooooooklyn]

Description:
xhr.onreadystatechange property may be monkey patched by other library such as zone.js.
It would cause AjaxObservable could not find subscriber, progressSubscriber and request property on xhr.onreadystatechange.

it's a simple example here: http://plnkr.co/edit/oX1v8tTjTog1T57Z0JnI?p=preview

[coveralls]

Coverage remained the same at 97.039% when pulling 00fc6b1 on Brooooooklyn:issue/ajax into 1bee98a on ReactiveX:master.

[coveralls]

Coverage remained the same at 97.039% when pulling 4f2f376 on Brooooooklyn:issue/ajax into 1bee98a on ReactiveX:master.

[coveralls]

Coverage decreased (-0.03%) to 97.009% when pulling 3466354 on Brooooooklyn:issue/ajax into 1bee98a on ReactiveX:master.

[coveralls]

Coverage decreased (-0.03%) to 97.009% when pulling 3466354 on Brooooooklyn:issue/ajax into 1bee98a on ReactiveX:master.

[benlesh]

@Brooooooklyn ... I'm struggling to understand the issue. I think I get it... but can you create a test case as part of this PR that definitively shows the issue this is trying to correct? You don't need to use zones, maybe just monkey patch the handler or something?

It seems if there's a problem like this, the alternative solution would be to use the addEventHandler means of registering handlers.

[jayphelps]

@Blesh Indeed the issue is that zone.js does not respect the identity of the function you assign to the event callbacks. It wraps the function here so a new function created internally is the one you actually receive when we xhr.onreadystatechange.something = 123 etc.

IMO is solution is acceptable and effectively the same we'd need to do if we used xhr.addEventListener -- that is, we need to assign the function to a local scope var first, then use that local scope var to assign the props, independently of setting up the listener logic.

[coveralls]

Coverage increased (+0.2%) to 97.283% when pulling 5920621 on Brooooooklyn:issue/ajax into 1bee98a on ReactiveX:master.

[Brooooooklyn]

@Blesh I've added some test in a separate commit, could you please review it.

[coveralls]

Coverage increased (+0.2%) to 97.284% when pulling 807cb7b on Brooooooklyn:issue/ajax into 5460e77 on ReactiveX:master.

[benlesh]

Oh. I understand. Not sure why it wasn't sinking in the first time I looked at it.

[coveralls]

Coverage increased (+0.2%) to 97.284% when pulling 0101c52 on Brooooooklyn:issue/ajax into ece1b89 on ReactiveX:master.

[coveralls]

Coverage increased (+0.2%) to 97.284% when pulling 0101c52 on Brooooooklyn:issue/ajax into ece1b89 on ReactiveX:master.

[jayphelps]

LGTM

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.