Closed
Bug 664692
Opened 13 years ago
Closed 12 years ago
Calling `new WebSocket()` results in a TypeError: "WebSocket is not a constructor"
Categories
(Core :: Networking: WebSockets, defect)
Tracking
()
People
(Reporter: james, Assigned: nossralf)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [qa-])
Attachments
(1 file, 2 obsolete files)
18.56 KB,
patch
|
asa
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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
Updated•13 years ago
|
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 :(
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
![]() |
||
Comment 3•13 years ago
|
||
This is breaking sites (see bug 664423). So I think we need to fix this for Fx6...
Fredrik: Can you take this? Otherwise I will.
Assignee | ||
Comment 5•13 years ago
|
||
I have a patch renaming nsIWebSocket to nsIMozWebSocket in the works. Expect a first version today.
Assignee | ||
Comment 6•13 years ago
|
||
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)
Updated•13 years ago
|
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?
Assignee | ||
Comment 8•12 years ago
|
||
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.
Updated•12 years ago
|
Keywords: dev-doc-needed
Comment 10•12 years ago
|
||
I thought there was a way to hide the feature from detection entirely (like document.all).
Why would we want to do that?
Comment 12•12 years ago
|
||
(In reply to comment #11) > Why would we want to do that? Sorry, missed the previous renaming bug. I completely misunderstood this.
Assignee | ||
Comment 13•12 years ago
|
||
Should this be checked in before the merge of aurora to beta happens? From what I could find, the merge happens tomorrow.
Updated•12 years ago
|
Keywords: checkin-needed
The patch is bitrotted.
Keywords: checkin-needed
Assignee | ||
Comment 15•12 years ago
|
||
Against current tip on aurora branch.
Attachment #541297 -
Attachment is obsolete: true
Attachment #543884 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
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?
![]() |
||
Comment 18•12 years ago
|
||
We just never prefixed on m-c, but we may change that in bug 659324....
Updated•12 years ago
|
Comment 19•12 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+
Comment 20•12 years ago
|
||
(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
Updated•12 years ago
|
Attachment #543884 -
Flags: approval-mozilla-aurora+ → approval-mozilla-beta?
Updated•12 years ago
|
Attachment #543884 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•12 years ago
|
Keywords: checkin-needed
Updated•12 years ago
|
Attachment #540771 -
Attachment is obsolete: true
Comment 21•12 years ago
|
||
http://hg.mozilla.org/releases/mozilla-beta/rev/8aa062d900d9
Keywords: checkin-needed
Comment 22•12 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•12 years ago
|
||
In latest fx6 beta WebSocket in not defined, but I have MozWebSocket constructor. Is this final api?
Reporter | ||
Comment 24•12 years ago
|
||
This appears fixed in latest FF6 beta channel.
Comment 25•12 years ago
|
||
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)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 26•12 years ago
|
||
Docs updated: https://developer.mozilla.org/en/WebSockets#Browser_compatibility Change also mentioned on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete
status-firefox7:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•