785319 - Setting WebSocket.binaryType to an invalid value should no longer throw

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://html5.org/tools/web-apps-tracker?from=7260&to=7261

nsWebSocket::SetBinaryType should return NS_OK instead of NS_ERROR_INVALID_ARG when an unknown value is passes. The code is here:

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsWebSocket.cpp#1213

Comment 1

[Abhishek Potnis [:avp]]

Hi Ms2ger,

I am interested in working on this bug. Could you please guide me on getting started on this bug.....



Thanks.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Hi there! To fix this bug, you'll need to remove the else branch at <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsWebSocket.cpp#1213>, and update the test at <http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_websocket.html?force=1#1282>.

If you need any help setting up a build environment etc., see <https://developer.mozilla.org/en-US/docs/Introduction> or come hang around on irc.mozilla.org, #introduction.

Comment 3

[Abhishek Potnis [:avp]]

Attachment: removed the else branch in file nsWebSocket.cpp

Comment 4

[Abhishek Potnis [:avp]]

Attachment: edited test_websocket.html

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 655053 [details] [diff] [review]
removed the else branch in file nsWebSocket.cpp

Review of attachment 655053 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 655054 [details] [diff] [review]
edited test_websocket.html

Looks good too.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Abhishek, could you generate a single patch following the instructions at <https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F>? That would be helpful to get it checked in.

Comment 8

[Abhishek Potnis [:avp]]

Attachment: patch

Comment 9

[Abhishek Potnis [:avp]]

Attachment: edited nsWebSocket.cpp and test_websocket.html

Comment 10

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d15fd2146d

Try results: https://tbpl.mozilla.org/?tree=Try&rev=fbe522587fcc

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e4d15fd2146d