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

Added sendBinary method for WebSocket class. #116

Closed
wants to merge 1 commit into from

Conversation

kyubuns
Copy link
Contributor

@kyubuns kyubuns commented Oct 26, 2012

I want to send binary on websocket!

@kyubuns
Copy link
Contributor Author

kyubuns commented Oct 26, 2012

With implementation as you say,(WebSocket.send(ubyte[] data) and WebSocket.send(String data))
the function send not text but binary in past code like below.

WebSocket.send(cast(ubyte[])"foo");

Is it alright?

@s-ludwig
Copy link
Member

I hope that just now, the number of people using the old send() is countable with one hand and I would like to take that chance to still be able to make such a change without causing too much harm. But do you know the exact difference between the two on the JavaScript side? Is binary data received as a BinData object instead of a string?

@kyubuns
Copy link
Contributor Author

kyubuns commented Oct 26, 2012

I know, but websocket spec contains opcode.
hard-coding of opcode value(FrameOpcode.Text) is ugly.
I will send pull request which is your implement(two overloads of send) later.

@s-ludwig
Copy link
Member

I was just asking because I don't know how browsers handle binary messages. But I agree that the hard-coded value is wrong and this needs to be fixed.

@kyubuns
Copy link
Contributor Author

kyubuns commented Oct 26, 2012

for example:
var ws = new WebSocket("ws://localhost:9998");
ws.onmessage = function (e) {
console.debug(e.data.constructor); //print type of e.data
};

opcode==Text

function String() { [native code] }

opcode==Binary

function Blob() { [native code] }
function ArrayBuffer() { [native code] } //ws.binaryType = 'arraybuffer';

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.

2 participants