Skip to content
This repository has been archived by the owner on Jul 25, 2018. It is now read-only.

Fix critical error in target device javascript #8

Closed
wants to merge 1 commit into from

Conversation

pospi
Copy link

@pospi pospi commented Aug 29, 2014

This fixes javascript errors which cause the script to fail and results in targets never finding the weinre server.

Due to use of function.bind this does not work in IE8 but inclusion of a polyfill would be simple if desired.

@pmuellr
Copy link
Contributor

pmuellr commented Aug 29, 2014

pospi - thx for the PR. Can you open a bug so we can track this?

I guess this fix looks ok, but is a bit worrisome. Is the problem that the readystatechange handler is not passed an event on some platform, or that the event that it's passed doesn't have the xhr as the event target?

I assume the original code worked on some platform(s).

Also worried about about requiring function.bind(); perhaps we should "bind by hand" with an additional function with the this in a closure?

@pmuellr
Copy link
Contributor

pmuellr commented Aug 30, 2014

I've opened a bug for this:

https://issues.apache.org/jira/browse/CB-7437

I've got a slightly different patch, here:

https://github.com/apache/cordova-weinre/blob/CB-7437/weinre.web/modules/weinre/common/WebSocketXhr.coffee#L204-L215

Seems like it works to me, and doesn't require fn.bind().

I'll merge in a few days, let me know if you have any issues with it.

@pmuellr
Copy link
Contributor

pmuellr commented Aug 30, 2014

sorry, should page posted the commit as well:

@pospi
Copy link
Author

pospi commented Aug 31, 2014

Hey Patrick,

Hah, beat me to it! Thanks for that (: I was going to put this together when I next got back to the office - the device affected was an iPad 4 (I believe iOS 7.1) and that's the only place where I have access to one. I guess it either references the xhr object elsewhere on the event or doesn't provide it; either way wrapping the reference in a closure should cover all bases. thanks again

@pmuellr
Copy link
Contributor

pmuellr commented Aug 31, 2014

Curious why you're using weinre for iPad when you could be using Web Inspector. Does Web Inspector somehow not work in your environment?

https://developer.apple.com/library/mac/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/GettingStarted/GettingStarted.html

@pospi
Copy link
Author

pospi commented Aug 31, 2014

Oh it works just fine - provided you drink all the cool-aid and run OSX Mavericks or later. I refuse to so I wanted to be able to debug without having to go use somebody else's mac.

@jcesarmobile
Copy link
Member

Closing as this was already committed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants