gimite / web-socket-js

HTML5 Web Socket implementation powered by Flash
BSD 3-Clause "New" or "Revised" License
2.73k stars 489 forks source link

Minor syntax improvements + added pure .as support. #77

Closed ghost closed 13 years ago

ghost commented 13 years ago
  1. Fixed imports.
  2. Moved files to net.gimite as per convention.
  3. Made a few properties public.
  4. Removed hurlant and adobe code, replaced with lib swc as per convention.
  5. Added ant build, as per .as convention.
  6. Trapped externalInterface in constructor and try/catch. This enabled use in .as and Air projects. (There are 3MM flash developers)

I plan to send you an email.

ghost commented 13 years ago

I mentioned on Twitter that I'd like to clean up optimize some of the code. Majority of the changes is just standard formatting. I also enabled use w/ pure .as & Air, by trapping ExternalInterface. Sample use:

http://dl.dropbox.com/u/2127757/gimiteSrc.zip. I could add these files as well, so that the project works w/ .js as now + .as/Air support w/ example. Ex: It would work in IOSBrowser and IOSApp. I plan to push example to npm from www.gamina.org. My twitter name is puppetMaster3.

ghost commented 13 years ago

I closed by mistake.

ghost commented 13 years ago

The changes look big, they are not, you can see what it would look like after: https://github.com/puppetMaster3/web-socket-js

kanaka commented 13 years ago

I generally like the changes.

Some comments:

ghost commented 13 years ago

Thank you for your time.

  1. I have not tested TSL/SSL. Do you have some note suggestions on how to do that? Or is it easy for you? The hurlant swc is same as 'old' source, and in general I only changed minor format things so it should work. But it needs to be tested.

Other than that, I hope that the rest I can patch quickly after the push of these changes. I only learned a git to do this.

So please accept the push after we test ssl. I'll do other changes you mentioned right after, and discuss the other things further.

kanaka commented 13 years ago

I will see if I can find some time to test your changes in the next couple of days. I will just do it with https://github.com/kanaka/noVNC and https://github.com/kanaka/websockify which uses web-socket-js and has support for encrypted connections. Also, I have some WebSocket tests in websockify that I used for functionality and performance testing.

Pull and release issues are really up to gimite since he owns the main repository. But my personal opinion is that the issues should be cleaned up in your own repo and then re-issue the pull request.

The plan for the other protocol branch (according to previous conversation with gimite) is that once Chrome switches to the new protocol gimite will merge the changes from the hybi-07 branch and make it default. I think that's probably the right direction. Developers control the version of web-socket-js that they have with their application, so I don't think there is much point in having the ability to choose on the client side which version of the protocol to use. I.e. developers will just have the websocket server and the web-socket-js version stay in sync (same with the insecure version).

If you are building from the hurlant sources that are currently in the tree, you probably shouldn't delete them yet since there is no way for somebody checking out the repo to rebuild the *.swc libraries (without poking back into git history that is).

A pure .as example would be good.

IMO, having release and development branches (after all, forks serve as development branches too with git) is probably overkill. web-socket-js isn't really the sort of thing where multiple versions are a big need. But that's just my opinion. gimite makes the final call.

ghost commented 13 years ago

Thx again Kanaka. I would be more than happy to do all that before as well in short order. But I don't want to spin my wheels either. I'd like to hear from gimite / Hiroshi as to what it takes to accept the commit/pull. This would avoid me wasting time. All your items make sense, I'll do them after a comment from gimite /Hiroshi. Lets home we hear back.

gimite commented 13 years ago

Thanks for the patch. Sorry for late. I applied some of your changes.

Fixed imports.

Applied.

Moved files to net.gimite as per convention.

Applied, but to net.gimite.websocket instead.

Made a few properties public. Trapped externalInterface in constructor and try/catch. This enabled use in .as and Air projects. (There are 3MM flash developers)

To use this library from ActionScript, I suggest to use WebSocket class directly, not WebSocketMain. Role of WebSocketMain is to bridge between WebSocket class and JavaScript. You need to define your own logger implementing IWebSocketLogger, but otherwise it should be straight forward to use WebSocket class directly. It should be documented somewhere, though...

So I kept WebSocketMain.as untouched.

Removed hurlant and adobe code, replaced with lib swc as per convention.

I prefer to keep original source code instead of intermediate file in repository when possible. The reason why I keep WebSocketMain.swf is to make the library easy to use. So I'll keep .as files.

Added ant build, as per .as convention.

Done, with some modification. I also kept build.sh because it's simpler and has less dependencies, as @kanaka mentioned.

Please let me know if you have any further suggestion. Otherwise I'll apply the same thing to hybi-07 branch.

gimite commented 13 years ago
  • Same for insecure SWF, it can be a single swf 'useInsecure' can be exposed. I don't care what the methods are called, but it seems it be easier to add that.

Important point is that WebSocketInsecure.swf is insecure, so its feature should be exposed to Web only when the developer decide to use it. That's why it's separated SWF and in ZIP. Please let me know if you have better idea meeting that criteria.

  • I'd like to add a pure .as example for flash/flex/air in a package here.

That would be nice. As I said in previous post, I suggest to use WebSocket class directly.

  • Consider 'branching on release'. IE. Trunk is 'unstable dev'. Once it is stable, it goes to a branch ex: branchV2.1. We can start soon or even now.

As @kanaka mentioned, I guess it's too much. I don't expect much big changes except for WebSocket protocol version update. It might be better to have some versioning to make it easier for users to track updates, though...