Bug 110601

Summary: [WebSocket] it is better to send User-Agent in opening handshakes
Product: WebKit Reporter: Takashi Toyoshima <toyoshim>
Component: New BugsAssignee: Takashi Toyoshima <toyoshim>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Takashi Toyoshima 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 Takashi Toyoshima 2013-02-22 05:57:13 PST
Created attachment 189762 [details]
Patch
Comment 2 Takashi Toyoshima 2013-02-22 06:36:17 PST
Created attachment 189766 [details]
Patch
Comment 3 Takashi Toyoshima 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 Alexey Proskuryakov 2013-02-22 09:26:11 PST
Comment on attachment 189766 [details]
Patch

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 Takashi Toyoshima 2013-02-25 21:26:39 PST
Comment on attachment 189766 [details]
Patch

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 Takashi Toyoshima 2013-02-25 21:30:34 PST
Created attachment 190198 [details]
Patch
Comment 7 WebKit Review Bot 2013-02-26 04:03:17 PST
Comment on attachment 190198 [details]
Patch

Clearing flags on attachment: 190198

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