Bug 102889 – WebSocket's MessageEvent.origin attribute is an empty string

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

According to specification (http://www.w3.org/TR/2012/CR-websockets-20120920/#feedback-from-the-protocol), we should "initialize [message] event's origin attribute to the Unicode serialization of the origin of the URL that was passed to the WebSocket object's constructor.". However, the current implementation returns an empty string for the event's origin.

Firefox seems to populate MessageEvent.origin according to the specification.

[Ian 'Hixie' Hickson]

Note that that URL is out of date. The current spec for Web Sockets is at:
   http://www.whatwg.org/specs/web-apps/current-work/multipage/network.html#feedback-from-the-protocol

Basically you never want to be referring to /TR/ versions of specs because they're always going to be out of date, because they're just snapshots of the current work.

In this particular case the requirement in question hasn't changed since the W3C made that snapshot, though.

[Chris Dumez]

Created attachment 175753 [details]
Patch

[Kentaro Hara]

Comment on attachment 175753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175753&action=review

> LayoutTests/ChangeLog:15
> +        * http/tests/websocket/tests/hybi/send-once_wsh.py: Added.

What's this? Why can't you write a layout test using the current layout test framework?

> LayoutTests/http/tests/websocket/tests/hybi/send-once_wsh.py:2
> +# Copyright (C) 2009 Google Inc. All rights reserved.
> +# Copyright (C) 2012 Intel Corporation. All rights reserved.

Is this copyright correct? If this is a new file, the copyright would be intel's one.

> LayoutTests/http/tests/websocket/tests/hybi/send-onmessage-origin.html:20
> +var MESSAGE_TO_SEND = "This is the message to send to the server.";

Nit: messageToSend. WebKit uses camelCase variables even for constants.

> LayoutTests/http/tests/websocket/tests/hybi/send-onmessage-origin.html:38
> +    shouldBe("origin", "'ws://localhost:8880'");

Nit: You can use ShouldBeEqualToString("origin", "ws://localhost:8880")

[Chris Dumez]

Comment on attachment 175753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175753&action=review

>> LayoutTests/http/tests/websocket/tests/hybi/send-once_wsh.py:2
>> +# Copyright (C) 2012 Intel Corporation. All rights reserved.
> 
> Is this copyright correct? If this is a new file, the copyright would be intel's one.

Well, this is basically a simplified version of LayoutTests/http/tests/websocket/tests/hybi/send_wsh.py, which expects 1 message instead of 2. Since the original file was with Google copyright and the code is mostly the same, I kept the copyright.
Anyway, it does not really matter since you prefer I use existing infrastructure. I guess I'll use LayoutTests/http/tests/websocket/tests/hybi/send_wsh.py and send 2 messages.

[Kentaro Hara]

(In reply to comment #4)
> Anyway, it does not really matter since you prefer I use existing infrastructure. I guess I'll use LayoutTests/http/tests/websocket/tests/hybi/send_wsh.py and send 2 messages.

Sounds nicer.

[Chris Dumez]

Created attachment 175758 [details]
Patch

Take Haraken's feedback into consideration.

[Kentaro Hara]

Comment on attachment 175758 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175758&action=review

Looks OK.

> LayoutTests/http/tests/websocket/tests/hybi/send-onmessage-origin.html:21
> +var FirstMessageToSend = "This is the first message to send to the server.";
> +var SecondMessageToSend = "This is the second.";

Sorry for nit-picking. WebKit uses camelCase even for constants. So this could be firstMessageToSend and secondMessageToSend.

[Chris Dumez]

(In reply to comment #7)
> (From update of attachment 175758 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175758&action=review
> 
> Looks OK.
> 
> > LayoutTests/http/tests/websocket/tests/hybi/send-onmessage-origin.html:21
> > +var FirstMessageToSend = "This is the first message to send to the server.";
> > +var SecondMessageToSend = "This is the second.";
> 
> Sorry for nit-picking. WebKit uses camelCase even for constants. So this could be firstMessageToSend and secondMessageToSend.

My bad. I'll fix it.

[Chris Dumez]

Created attachment 175762 [details]
Patch for landing

Fixed camel case nit. Could someone please cq+ ?

[Kentaro Hara]

Comment on attachment 175762 [details]
Patch for landing

thanks

[WebKit Review Bot]

Comment on attachment 175762 [details]
Patch for landing

Clearing flags on attachment: 175762

Committed r135587: <http://trac.webkit.org/changeset/135587>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.