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

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, defect)

Description

[James Wheare]

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]

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) No longer reading bugmail consistently]

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 [:bzbarsky]]

This is breaking sites (see bug 664423).  So I think we need to fix this for Fx6...

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Fredrik: Can you take this? Otherwise I will.

Comment 5

[Fredrik Larsson (:nossralf)]

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

Comment 6

[Fredrik Larsson (:nossralf)]

Attachment: 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) No longer reading bugmail consistently]

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)]

Attachment: 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) No longer reading bugmail consistently]

Mochitest is perfect and what we want here.

Comment 10

[Nochum Sossonko [:Natch]]

I thought there was a way to hide the feature from detection entirely (like document.all).

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Why would we want to do that?

Comment 12

[Nochum Sossonko [:Natch]]

(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)]

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 (Exited; not receiving bugmail, old account, do not use)]

The patch is bitrotted.

Comment 15

[Fredrik Larsson (:nossralf)]

Attachment: Rename interface, add tests: unbitrotted

Against current tip on aurora branch.

Comment 16

[Dão Gottwald [:dao]]

no approval -> removing checkin-needed

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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 [:bzbarsky]]

We just never prefixed on m-c, but we may change that in bug 659324....

Comment 19

[Asa Dotzler [:asa]]

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]]

(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 21

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-beta/rev/8aa062d900d9

Comment 22

[AndreiD[QA]]

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]

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

Comment 24

[James Wheare]

This appears fixed in latest FF6 beta channel.

Comment 25

[Patrick McManus [:mcmanus]]

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]]

Docs updated:

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

Change also mentioned on Firefox 6 for developers.

Comment 27

[u279076]

qa- as no QA fix verification needed