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

The WebSocketTask treats text messages as binary when using a web-sys. #1002

Closed
1 of 3 tasks
dinAlt opened this issue Mar 7, 2020 · 5 comments · Fixed by #1005
Closed
1 of 3 tasks

The WebSocketTask treats text messages as binary when using a web-sys. #1002

dinAlt opened this issue Mar 7, 2020 · 5 comments · Fixed by #1005
Labels

Comments

@dinAlt
Copy link

dinAlt commented Mar 7, 2020

Function did_process_binary in src/services/websocket.rs returns true even if an actual message data was a text, but resulting Vec, which passed as Option to OUT::from(data) is empty.

Environment:

  • Yew version 0.13.2
  • Rust version 1.41.1
  • web-sys version 0.3.36
  • OS: linux
  • Browser chromium
  • Browser version 80

A potential fix is to return false in did_process_binary if result of Uint8Array::new(&bytes).to_vec() is empty.

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later
@jstarry
Copy link
Member

jstarry commented Mar 8, 2020

Thanks for the bug report! I went ahead and fixed this myself because I've been meaning to add test coverage anyways.

@dinAlt
Copy link
Author

dinAlt commented Mar 8, 2020

Ok, should i close an issue, or do anything else now? I'm just not very good in github collaboration...

@jstarry
Copy link
Member

jstarry commented Mar 8, 2020

No need, once the PR is merged, the issue will be auto-closed

@jstarry
Copy link
Member

jstarry commented Mar 8, 2020

If you have the time, feel free to look over my changes to see if they are correct

@dinAlt
Copy link
Author

dinAlt commented Mar 8, 2020

Yes, text messages are works now, and your fix is definitely better, than just checking a vec length)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants