Closed Bug 1147990 Opened 5 years ago Closed 5 years ago

Pass W3C websocket test suite

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bzbarsky, Assigned: baku)

References

()

Details

Attachments

(4 files, 4 obsolete files)

Assignee: nobody → amarchesini
Attached patch 01_ref.patch (obsolete) — Splinter Review
new WebSocket("ws://something/#") should fail.
Attachment #8585574 - Flags: review?(bugs)
Attached patch 02_url.patch (obsolete) — Splinter Review
The default URL when the socket is not connected should be parsed.
Attachment #8585575 - Flags: review?(bugs)
Attached patch 03_port.patch (obsolete) — Splinter Review
Security error if the port is not allowed.
Attachment #8585576 - Flags: review?(bugs)
Comment on attachment 8585574 [details] [diff] [review]
01_ref.patch

Review of attachment 8585574 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/WebSocket.cpp
@@ +1867,5 @@
>  
> +  bool hasRef;
> +  rv = parsedURL->GetHasRef(&hasRef);
> +  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_SYNTAX_ERR);
> +  NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && !hasRef,

Don't need to check rv twice
Attached patch 01_ref.patch (obsolete) — Splinter Review
Attachment #8585574 - Attachment is obsolete: true
Attachment #8585574 - Flags: review?(bugs)
Attachment #8585620 - Flags: review?(bugs)
Attachment #8585575 - Flags: review?(bugs) → review+
Attachment #8585576 - Flags: review?(bugs) → review+
Attachment #8585620 - Flags: review?(bugs) → review+
Attached patch patch 1Splinter Review
Attachment #8585620 - Attachment is obsolete: true
Attached patch patch 2Splinter Review
Attachment #8585575 - Attachment is obsolete: true
Attached patch patch 3Splinter Review
Attachment #8585576 - Attachment is obsolete: true
Patches ready to land, if this is green enough:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab658a9d577b
Keywords: checkin-needed
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8275103&repo=mozilla-inbound
Flags: needinfo?(amarchesini)
Here the report about the webplatform tests:

/websockets/Close-reason-unpaired-surrogates.htm -> bigger problem about Unicode
/websockets/Close-undefined.htm -> The test is invalid - close(undefined) is OK
/websockets/Create-Secure-blocked-port.htm -> FIXED - patch 3
/websockets/Create-Secure-verify-url-set-non-default-port.htm -> the test has a JS error.
/websockets/constructor/002.html -> FIXED - patch 1
/websockets/constructor/010.html -> The test is invalid - webSocket protocol error
/websockets/constructor/018.html -> The test is invalid - no percentage replacement is required from the spec
/websockets/cookies/001.html -> Work-in-progress
/websockets/interfaces.html -> The test is invalid - no stringifier in the webidl definition
/websockets/interfaces/CloseEvent/historical.html -> No historical Close Event support
/websockets/interfaces/WebSocket/events/013.html -> This should be allowed by the spec.
/websockets/interfaces/WebSocket/readyState/003.html -> the test is invalid
/websockets/interfaces/WebSocket/url/001.html -> FIXED - patch 2
/websockets/opening-handshake/005.html -> The test is invalid - webSocket protocol error
/websockets/cookies/005.html -> The test is invalid - webSocket protocol error
/websockets/interfaces/WebSocket/close/close-connecting.html -> The test is invalid - webSocket not active
Attached patch patch 4Splinter Review
Attachment #8586090 - Flags: review?(james)
Attachment #8586090 - Flags: review?(james) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.