1147990 - Pass W3C websocket test suite

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

http://www.w3c-test.org/websockets/ has the tests

http://w3c.github.io/test-results/websockets/all.html has the results including which ones we fail.

https://lists.w3.org/Archives/Public/public-webapps/2015JanMar/0893.html has some comments about bugs in tests.

The three things I've seen that look like bugs on our side at first glance are:

http://www.w3c-test.org/websockets/interfaces/WebSocket/url/001.html
http://www.w3c-test.org/websockets/constructor/002.html
http://www.w3c-test.org/websockets/Create-Secure-blocked-port.htm

Comment 1

[Andrea Marchesini [:baku]]

Attachment: 01_ref.patch

new WebSocket("ws://something/#") should fail.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: 02_url.patch

The default URL when the socket is not connected should be parsed.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: 03_port.patch

Security error if the port is not allowed.

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

Comment 5

[Andrea Marchesini [:baku]]

Attachment: 01_ref.patch

Comment 6

[Andrea Marchesini [:baku]]

Attachment: patch 1

Comment 7

[Andrea Marchesini [:baku]]

Attachment: patch 2

Comment 8

[Andrea Marchesini [:baku]]

Attachment: patch 3

Comment 9

[Andrea Marchesini [:baku]]

Patches ready to land, if this is green enough:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab658a9d577b

Comment 10

[Andrea Marchesini [:baku]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/03bc64517aac
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d83e597679f5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5ddc5807af3e

Comment 11

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8275103&repo=mozilla-inbound

Comment 12

[Andrea Marchesini [:baku]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/dfb54485fbd5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8f9658014080
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4d52dbe2277d

Sorry, I forgot to update the web-platform-tests results.

Comment 13

[Andrea Marchesini [:baku]]

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

Comment 14

[Andrea Marchesini [:baku]]

Attachment: patch 4

Comment 15

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7cc272d5db

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dfb54485fbd5
https://hg.mozilla.org/mozilla-central/rev/8f9658014080
https://hg.mozilla.org/mozilla-central/rev/4d52dbe2277d
https://hg.mozilla.org/mozilla-central/rev/488aa4e61296
https://hg.mozilla.org/mozilla-central/rev/ee7cc272d5db