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

Remove invisible special characters at the beginning of websocket.js #400

Closed

Conversation

Honry
Copy link
Contributor

@Honry Honry commented Nov 4, 2013

No description provided.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/391

This is an external review system which you may optionally use for the code review of your pull request.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 4, 2013

It seems like any sofware that deals with the web will need to support BOMs, so I don't quite understand why this change is necessary.

@Honry
Copy link
Contributor Author

Honry commented Nov 5, 2013

I test the websocket case on my Crosswalk device, it raised “CreateWebSocket is not defined” exception, I found it loaded the websocket.js file failed in the case because of the invisible special characters at the beginning of websocket.js. And we remove it, then the test passes.

@Honry
Copy link
Contributor Author

Honry commented Nov 5, 2013

hi, Ms2ger

Could you please help explain "BOMs"? I'm not sure what it means.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 5, 2013

What you removed is the UTF-8 BOM.

@zqzhang
Copy link
Contributor

zqzhang commented Nov 7, 2013

Hi @Ms2ger, thank you for the comments. While I agree to 'any sofware that deals with the web will need to support BOMs', I still wonder if this character is a MUST for this file?

@sideshowbarker
Copy link
Contributor

@zqzhang Nobody said that having a BOM in this file is a must. Instead the point is that removing it is not a must. It's not necessary to remove it because there's nothing broken in the file. If Crosswalk in fact can't deal correctly with BOMs in JS files, then Crosswalk is broken and needs to be fixed.

@zqzhang
Copy link
Contributor

zqzhang commented Nov 7, 2013

@sideshowbarker, got it, thanks.

@Honry
Copy link
Contributor Author

Honry commented Nov 7, 2013

Thanks for your all comments, I will close this PR.

@Honry Honry closed this Nov 7, 2013
@Honry Honry deleted the Honry/fix-websockets-messy-code branch November 7, 2013 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants