Bug 110601 - [WebSocket] it is better to send User-Agent in opening handshakes
: [WebSocket] it is better to send User-Agent in opening handshakes
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2013-02-22 05:51 PST by
Modified: 2013-02-26 04:03 PST (History)


Attachments
Patch (2.18 KB, patch)
2013-02-22 05:57 PST, Takashi Toyoshima
no flags Review Patch | Details | Formatted Diff | Diff
Patch (7.69 KB, patch)
2013-02-22 06:36 PST, Takashi Toyoshima
no flags Review Patch | Details | Formatted Diff | Diff
Patch (7.63 KB, patch)
2013-02-25 21:30 PST, Takashi Toyoshima
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2013-02-22 05:51:50 PST
This is not mandatory, but spec allow to send HTTP compatible headers in opening handshake, and many web application developers want to have this.
------- Comment #1 From 2013-02-22 05:57:13 PST -------
Created an attachment (id=189762) [details]
Patch
------- Comment #2 From 2013-02-22 06:36:17 PST -------
Created an attachment (id=189766) [details]
Patch
------- Comment #3 From 2013-02-22 06:40:04 PST -------
Having layout test must be nice :)

Alexey, and Kent:
Can one of you take a look the second patch?
------- Comment #4 From 2013-02-22 09:26:11 PST -------
(From update of attachment 189766 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=189766&action=review

It's so horrible that we have to build the header twice, once for real and second time for Web Inspector.

r=me

> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

Please use a regular HTML5 doctype:

<!DOCTYPE html>

> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:4
> +<script src="../../../../js-test-resources/js-test-pre.js"></script>

There is no need to build a document relative path, you can just use 'src="/js-test-resources/js-test-pre.js"'.

> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:27
> +    data = messageEvent.data;
> +    useragent = navigator.userAgent;
> +    shouldBe("data", "useragent");

Why not shouldBe("messageEvent.data", "navigator.userAgent")? That would make test output a little clearer.

> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:40
> +<script src="../../../../js-test-resources/js-test-post.js"></script>

Ditto.
------- Comment #5 From 2013-02-25 21:26:39 PST -------
(From update of attachment 189766 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=189766&action=review

>> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:1
>> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> 
> Please use a regular HTML5 doctype:
> 
> <!DOCTYPE html>

Fixed.

>> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:4
>> +<script src="../../../../js-test-resources/js-test-pre.js"></script>
> 
> There is no need to build a document relative path, you can just use 'src="/js-test-resources/js-test-pre.js"'.

Fixed.

>> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:27
>> +    shouldBe("data", "useragent");
> 
> Why not shouldBe("messageEvent.data", "navigator.userAgent")? That would make test output a little clearer.

messageEvent can not be used directly because global variable is required to be compared correctly by shouldBe().
But your suggestion is very reasonable. I use navigator.userAgent directly, and assign messageEvent to global variable event, then use event.data to compare.

>> LayoutTests/http/tests/websocket/tests/hybi/useragent-in-openinghandshake.html:40
>> +<script src="../../../../js-test-resources/js-test-post.js"></script>
> 
> Ditto.

Fixed.
------- Comment #6 From 2013-02-25 21:30:34 PST -------
Created an attachment (id=190198) [details]
Patch
------- Comment #7 From 2013-02-26 04:03:17 PST -------
(From update of attachment 190198 [details])
Clearing flags on attachment: 190198

Committed r144037: <http://trac.webkit.org/changeset/144037>
------- Comment #8 From 2013-02-26 04:03:21 PST -------
All reviewed patches have been landed.  Closing bug.