Calling `new WebSocket()` results in a TypeError: "WebSocket is not a constructor"




Networking: WebSockets
6 years ago
5 years ago


(Reporter: James Wheare, Assigned: nossralf)



Dependency tree / graph

Firefox Tracking Flags

(firefox6+ fixed, firefox7+ fixed)


(Whiteboard: [qa-])


(1 attachment, 2 obsolete attachments)



6 years ago
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://');

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

6 years ago
Regression window:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a2) Gecko/20110606 Firefox/6.0a2 ID:20110609105104
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a2) Gecko/20110609 Firefox/6.0a2 ID:20110609161545
Triggered by:
b22e6569813	Fredrik Larsson — Bug 659324. Add Moz prefix to the WebSocket constructor. r=sicking a=asa


6 years ago
Blocks: 659324
OS: Mac OS X → All
Component: General → Networking: WebSockets
Product: Firefox → Core
QA Contact: general → networking.websockets
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 :(


6 years ago
Ever confirmed: true
This is breaking sites (see bug 664423).  So I think we need to fix this for Fx6...
Blocks: 664423
tracking-firefox6: --- → ?
tracking-firefox7: --- → ?
Fredrik: Can you take this? Otherwise I will.

Comment 5

6 years ago
I have a patch renaming nsIWebSocket to nsIMozWebSocket in the works. Expect a first version today.

Comment 6

6 years ago
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.
Assignee: nobody → nossralf+bugzilla
Attachment #540771 - Flags: review?(jonas)
tracking-firefox6: ? → +
Attachment #540771 - Flags: review?(jonas) → review+
Can you also add a test that checks that

|"MozWebSocket" in window| returns true and |"WebSocket" in window| returns false?

Comment 8

6 years ago
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.
Mochitest is perfect and what we want here.
Keywords: dev-doc-needed
I thought there was a way to hide the feature from detection entirely (like document.all).
Why would we want to do that?
(In reply to comment #11)
> Why would we want to do that?

Sorry, missed the previous renaming bug. I completely misunderstood this.

Comment 13

6 years ago
Should this be checked in before the merge of aurora to beta happens? From what I could find, the merge happens tomorrow.


6 years ago
Keywords: checkin-needed
The patch is bitrotted.
Keywords: checkin-needed

Comment 15

6 years ago
Created attachment 543884 [details] [diff] [review]
Rename interface, add tests: unbitrotted

Against current tip on aurora branch.
Attachment #541297 - Attachment is obsolete: true
Attachment #543884 - Flags: approval-mozilla-aurora?


6 years ago
Keywords: checkin-needed
no approval -> removing checkin-needed
Keywords: checkin-needed
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.
Attachment #543884 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
We just never prefixed on m-c, but we may change that in bug 659324....


6 years ago
tracking-firefox7: ? → +

Comment 19

6 years ago
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.
Attachment #543884 - Flags: approval-mozilla-beta? → approval-mozilla-aurora+
(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.
status-firefox6: --- → affected


6 years ago
Attachment #543884 - Flags: approval-mozilla-aurora+ → approval-mozilla-beta?


6 years ago
Attachment #543884 - Flags: approval-mozilla-beta? → approval-mozilla-beta+


6 years ago
Keywords: checkin-needed


6 years ago
Attachment #540771 - Attachment is obsolete: true
status-firefox6: affected → fixed
Keywords: checkin-needed

Comment 22

6 years ago
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

6 years ago
In latest fx6 beta WebSocket in not defined, but I have MozWebSocket constructor.
Is this final api?

Comment 24

6 years ago
This appears fixed in latest FF6 beta channel.
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)
Last Resolved: 6 years ago
Resolution: --- → FIXED
Docs updated:

Change also mentioned on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete


6 years ago
status-firefox7: --- → fixed
qa- as no QA fix verification needed
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.