Last Comment Bug 664692 - Calling `new WebSocket()` results in a TypeError: "WebSocket is not a constructor"
: Calling `new WebSocket()` results in a TypeError: "WebSocket is not a constru...
Status: RESOLVED FIXED
[qa-]
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: unspecified
: x86 All
: -- normal with 2 votes (vote)
: ---
Assigned To: Fredrik Larsson (:nossralf)
:
Mentors:
Depends on:
Blocks: 659324 664423
  Show dependency treegraph
 
Reported: 2011-06-16 04:26 PDT by James Wheare
Modified: 2012-05-30 07:15 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed


Attachments
Rename nsIWebSocket to nsIMozWebSocket (16.95 KB, patch)
2011-06-21 09:18 PDT, Fredrik Larsson (:nossralf)
jonas: review+
Details | Diff | Review
Adds test for feature detection (18.55 KB, patch)
2011-06-22 23:47 PDT, Fredrik Larsson (:nossralf)
no flags Details | Diff | Review
Rename interface, add tests: unbitrotted (18.56 KB, patch)
2011-07-04 23:36 PDT, Fredrik Larsson (:nossralf)
asa: approval‑mozilla‑beta+
Details | Diff | Review

Description James Wheare 2011-06-16 04:26:33 PDT
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_7) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.91 Safari/534.30
Build Identifier: aurora 6.0a2

There appears to be a WebSocket object available but it's not callable according to the WebSocket spec. This breaks common feature detection code:

    if ('WebSocket' in window)

and requires an extra check before trying to create a WebSocket:

    if ('WebSocket' in window && typeof WebSocket == 'function')

Not sure if this is because WebSockets are still disabled by default until the spec issue clears, but if that's the case then the WebSocket object shouldn't exist at all so that common existing feature detection isn't broken. Unless of course there's a valid reason for the WebSocket object to exist without being callable. As far as I can tell it just contains a map of the ready states.


Reproducible: Always

Steps to Reproduce:
if ('WebSocket' in window) {
    new WebSocket('ws://example.com');
}


Actual Results:  
TypeError("WebSocket is not a constructor")

Expected Results:  
Either nothing if 'WebSocket' is not in window, or a working WebSocket object if it is.
Comment 1 Alice0775 White 2011-06-16 06:01:08 PDT
Regression window:
Works:
http://hg.mozilla.org/releases/mozilla-aurora/rev/c86edc448a83
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a2) Gecko/20110606 Firefox/6.0a2 ID:20110609105104
Fails:
http://hg.mozilla.org/releases/mozilla-aurora/rev/0b22e6569813
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a2) Gecko/20110609 Firefox/6.0a2 ID:20110609161545
Pushlog:
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=c86edc448a83&tochange=0b22e6569813
Triggered by:
b22e6569813	Fredrik Larsson — Bug 659324. Add Moz prefix to the WebSocket constructor. r=sicking a=asa
Comment 2 Jonas Sicking (:sicking) 2011-06-16 09:28:19 PDT
The "WebSocket in window" test should not return true.

I think what we need to do here is go through and rename the interface from nsIWebSocket to nsIMozWebSocket everywhere :(
Comment 3 Boris Zbarsky [:bz] 2011-06-20 19:19:27 PDT
This is breaking sites (see bug 664423).  So I think we need to fix this for Fx6...
Comment 4 Jonas Sicking (:sicking) 2011-06-20 23:19:45 PDT
Fredrik: Can you take this? Otherwise I will.
Comment 5 Fredrik Larsson (:nossralf) 2011-06-20 23:33:49 PDT
I have a patch renaming nsIWebSocket to nsIMozWebSocket in the works. Expect a first version today.
Comment 6 Fredrik Larsson (:nossralf) 2011-06-21 09:18:10 PDT
Created attachment 540771 [details] [diff] [review]
Rename nsIWebSocket to nsIMozWebSocket

This patch includes renaming the file nsIWebSocket.idl to nsIMozWebSocket.idl. I did not rename the nsWebSocket class to nsMozWebSocket, I felt it unnecessary since this is a temporary solution and changes only the web facing interface.

Please note: I am having Python issues with the WebSocket test server on my computer, so I haven't been able to run all of the WebSocket mochitests. A try server spin is probably a good idea.

However, I have manually verified (using the Web Developer JS console)  that |'WebSocket' in window| now returns false as intended, and that |'MozWebSocket' in window| returns true.
Comment 7 Jonas Sicking (:sicking) 2011-06-22 18:18:35 PDT
Can you also add a test that checks that

|"MozWebSocket" in window| returns true and |"WebSocket" in window| returns false?
Comment 8 Fredrik Larsson (:nossralf) 2011-06-22 23:47:34 PDT
Created attachment 541297 [details] [diff] [review]
Adds test for feature detection

Added a test to exercise the feature detection logic. Made it a Mochitest, not quite sure if that's the best place for it (IIRC |window| isn't available in xpcshell, for example). Let me know if it should be a different kind of test instead.
Comment 9 Jonas Sicking (:sicking) 2011-06-23 11:26:44 PDT
Mochitest is perfect and what we want here.
Comment 10 Nochum Sossonko [:Natch] 2011-06-27 11:32:03 PDT
I thought there was a way to hide the feature from detection entirely (like document.all).
Comment 11 Jonas Sicking (:sicking) 2011-06-27 12:36:51 PDT
Why would we want to do that?
Comment 12 Nochum Sossonko [:Natch] 2011-06-27 12:45:06 PDT
(In reply to comment #11)
> Why would we want to do that?

Sorry, missed the previous renaming bug. I completely misunderstood this.
Comment 13 Fredrik Larsson (:nossralf) 2011-07-04 08:55:07 PDT
Should this be checked in before the merge of aurora to beta happens? From what I could find, the merge happens tomorrow.
Comment 14 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-04 22:57:26 PDT
The patch is bitrotted.
Comment 15 Fredrik Larsson (:nossralf) 2011-07-04 23:36:40 PDT
Created attachment 543884 [details] [diff] [review]
Rename interface, add tests: unbitrotted

Against current tip on aurora branch.
Comment 16 Dão Gottwald [:dao] 2011-07-05 03:13:57 PDT
no approval -> removing checkin-needed
Comment 17 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-05 06:29:58 PDT
Comment on attachment 543884 [details] [diff] [review]
Rename interface, add tests: unbitrotted

Thanks for updating the patch.  This isn't going to make the cutoff, so asking for approval for beta.

Does this need to land for anything other than Firefox 6?  It looks like we unprefixed at some point.
Comment 18 Boris Zbarsky [:bz] 2011-07-05 11:10:38 PDT
We just never prefixed on m-c, but we may change that in bug 659324....
Comment 19 Asa Dotzler [:asa] 2011-07-07 14:48:34 PDT
Comment on attachment 543884 [details] [diff] [review]
Rename interface, add tests: unbitrotted

this needs to land on aurora and central. it's covered on beta.
Comment 20 Dão Gottwald [:dao] 2011-07-08 12:39:01 PDT
(In reply to comment #19)
> Comment on attachment 543884 [details] [diff] [review] [review]
> Rename interface, add tests: unbitrotted
> 
> this needs to land on aurora and central. it's covered on beta.

No, it isn't. In fact, it can only land on beta right now, because bug 659324 isn't on aurora / central.
Comment 22 AndreiD[QA] 2011-07-14 04:48:26 PDT
Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0

Is this fixed on beta since the patch landed(comment 21)?
Comment 23 Alexey Androsov 2011-07-19 03:25:48 PDT
In latest fx6 beta WebSocket in not defined, but I have MozWebSocket constructor.
Is this final api?
Comment 24 James Wheare 2011-07-20 09:59:55 PDT
This appears fixed in latest FF6 beta channel.
Comment 25 Patrick McManus [:mcmanus] 2011-07-25 12:57:18 PDT
this is really a missing piece of 659324, and as that patch has to deal with bitrot I am just going to include it in the update of 659324 so it does not get lost or placed in different locations.

will mark this bug as fixed, because it checked in the patch it was supposed to to fix the broken branch (comment 21)
Comment 26 Eric Shepherd [:sheppy] 2011-08-10 13:27:54 PDT
Docs updated:

https://developer.mozilla.org/en/WebSockets#Browser_compatibility

Change also mentioned on Firefox 6 for developers.
Comment 27 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 15:10:28 PDT
qa- as no QA fix verification needed

Note You need to log in before you can comment on or make changes to this bug.