mkuklis / phonegap-websocket

Websocket PhoneGap plugin for Android
203 stars 78 forks source link

Use native WebSockets on KitKat #23

Closed dhuyvett closed 10 years ago

dhuyvett commented 10 years ago

Added check to see whether WebSockets are available. Also added global boolean usingCordovaWebSocket. Unfortunately, native WebSockets on KitKat do not tolerate the options object in the constructor. Client code that uses the options object will have to do something like:

if (usingCordovaWebSocket) {

    socket = new WebSocket("ws://10.0.2.2:9003", "", { draft : "draft17" });

} else {

    socket = new WebSocket("ws://10.0.2.2:9003");
}
juniorz commented 10 years ago

@mkuklis In my case (Nexus 4 running Kit Kat from factory image) WebSocket.length is undefined when running the app with the plugin installed.

mkuklis commented 10 years ago

@juniorz that's very strange. What do you see when you do console.log(WebSocket) then?

Maybe the condition here should be modified to:


if (Websocket && Websocket.length) {
 // do your stuff...
}
juniorz commented 10 years ago
    console.log('WebSocket' + WebSocket);
    console.log('WebSocket arity = ' + WebSocket.length);

returns

WebSocket[object Object] WebSocket arity = undefined

mkuklis commented 10 years ago

Thank you @juniorz Something very weird is going on. Are you using 3.x.x version? Thx. The output of console.log(WebSocket) should be a function. I wonder if Phonegap's clobbers is screwing things up for us. I don't have access to KitKat here but could you give me a favor and remove this line: https://github.com/mkuklis/phonegap-websocket/blob/master/plugin.xml#L27 and try again? We will see what happens then.

juniorz commented 10 years ago

I've just updated cordova to 3.1.0-0.2.0 (it was 3.1.0-0.1.0 previously) as mentioned here but git diff doesn't show anything updated.

And I've also updated the plugin by removing and reinstalling it (in an attempt to make sure this PR is included in my version of the plugin).

juniorz commented 10 years ago

I'm also downloading Android 4.4 sdk to see if that might be the issue.

mkuklis commented 10 years ago

Sounds good. Please try to remove https://github.com/mkuklis/phonegap-websocket/blob/master/plugin.xml#L27 from the plugin and run cordova prepare android to make sure the change is included in platforms\android. I would be curious to see what you with get from console.log(WebSocket) then.

juniorz commented 10 years ago

I removed the plugin, and the socket works perfectly. ¬¬

mkuklis commented 10 years ago

Right because KitKat is based on Chrome which has WebSocket support so you don't need the plugin at all.

mkuklis commented 10 years ago

This line should take care of that: https://github.com/mkuklis/phonegap-websocket/blob/master/www/phonegap-websocket.js#L3

juniorz commented 10 years ago

But what if I want to support older devices?

Without the plugin, the output is

"WebSocketfunction WebSocket() { [native code] }" "WebSocket arity = 1"

mkuklis commented 10 years ago

@juniorz that's why you need the plugin (to support older android versions). That's why we have this: https://github.com/mkuklis/phonegap-websocket/blob/master/www/phonegap-websocket.js#L3 in older versions the first condition will fail and it will fall back on the plugin implementation. Could you please try to do what I described above? Thx.

juniorz commented 10 years ago

I think there is a typo on that line, isn't?

var Websocket = window.WebSocket = function(url, protocols, options)

Shouldn't it be var WebSocket = window.WebSocket = function(url, protocols, options) ?? (note the lower-cased s in the first variable declaration).

The code starts defining a Websocket var and then, exports WebSocket

mkuklis commented 10 years ago

You are right! that could be the problem! Do you want to send a pull request?

juniorz commented 10 years ago

I'm trying both first, and will send a PR if that is the case.

mkuklis commented 10 years ago

Awesome thank you for spotting this typo! I think PR is needed regardless :)

juniorz commented 10 years ago

I removed the <clobbers target="WebSocket" /> and it worked.

mkuklis commented 10 years ago

So it seems like if we don't export WebSocket via module.exports PhoneGap just exposes some empty object. Let me do a little change here and would you mind trying it again? I will let you know when it's ready.

juniorz commented 10 years ago

Sure.

mkuklis commented 10 years ago

Ok I just made a little change: https://github.com/mkuklis/phonegap-websocket/commit/843672ba96be110c6f9df9aa2bb75c63e30ca80d Would you mind trying it again with clobbers present? You can just reinstall the plugin again. Thx!

juniorz commented 10 years ago

Perfect! :shipit:

mkuklis commented 10 years ago

Great! I learned something new here too :)