Bug 110601

Summary:

[WebSocket] it is better to send User-Agent in opening handshakes

Product:

WebKit

Reporter:

Takashi Toyoshima <toyoshim>

Component:

New Bugs

Assignee:

Takashi Toyoshima <toyoshim>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Takashi Toyoshima

Reported 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.

Attachments
Patch (2.18 KB, patch) 2013-02-22 05:57 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
Patch (7.69 KB, patch) 2013-02-22 06:36 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
Patch (7.63 KB, patch) 2013-02-25 21:30 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Takashi Toyoshima

Comment 1 2013-02-22 05:57:13 PST

Created attachment 189762 [details] Patch

Takashi Toyoshima

Comment 2 2013-02-22 06:36:17 PST

Created attachment 189766 [details] Patch

Takashi Toyoshima

Comment 3 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?

Alexey Proskuryakov

Comment 4 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.

Takashi Toyoshima

Comment 5 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.

Takashi Toyoshima

Comment 6 2013-02-25 21:30:34 PST

Created attachment 190198 [details] Patch

WebKit Review Bot

Comment 7 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>

WebKit Review Bot

Comment 8 2013-02-26 04:03:21 PST

All reviewed patches have been landed. Closing bug.

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