Closed Bug 664692 Opened 13 years ago Closed 13 years ago

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


(Core :: Networking: WebSockets, defect)

Not set



Tracking Status
firefox6 + fixed
firefox7 + fixed


(Reporter: james, Assigned: nossralf)



(Keywords: dev-doc-complete, Whiteboard: [qa-])


(1 file, 2 obsolete files)

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.
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
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 :(
Ever confirmed: true
This is breaking sites (see bug 664423).  So I think we need to fix this for Fx6...
Blocks: 664423
Fredrik: Can you take this? Otherwise I will.
I have a patch renaming nsIWebSocket to nsIMozWebSocket in the works. Expect a first version today.
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)
Can you also add a test that checks that

|"MozWebSocket" in window| returns true and |"WebSocket" in window| returns false?
Attached patch Adds test for feature detection (obsolete) — Splinter Review
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.
I thought there was a way to hide the feature from detection entirely (like document.all).
(In reply to comment #11)
> Why would we want to do that?

Sorry, missed the previous renaming bug. I completely misunderstood this.
Should this be checked in before the merge of aurora to beta happens? From what I could find, the merge happens tomorrow.
Keywords: checkin-needed
Against current tip on aurora branch.
Attachment #541297 - Attachment is obsolete: true
Attachment #543884 - Flags: approval-mozilla-aurora?
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....
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.
Attachment #543884 - Flags: approval-mozilla-aurora+ → approval-mozilla-beta?
Attachment #543884 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: checkin-needed
Attachment #540771 - Attachment is obsolete: true
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)?
In latest fx6 beta WebSocket in not defined, but I have MozWebSocket constructor.
Is this final api?
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)
Closed: 13 years ago
Resolution: --- → FIXED
Docs updated:

Change also mentioned on Firefox 6 for developers.
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.