Bug 63141

Summary: Wrong argument order in window.postMessage
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, andersca, ap, arv, ian, johnjbarton, paulirish, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Description Yury Semikhatsky 2011-06-22 08:27:06 PDT
The specification says(http://dev.w3.org/html5/postmsg/#posting-messages) that the order of window.postMessage arguments is (message, targetOrigin, ports) while current WebKit implementation expects optional ports argument to go second i.e. (message, ports, targetOrigin). It was implemented this way in WebKit a couple of years ago(http://trac.webkit.org/changeset/32597/trunk/WebCore/bindings/js/JSDOMWindowCustom.cpp) so I'm not sure what we should do about this now when there may be some uses of that implementation.
Comment 1 Yury Semikhatsky 2011-06-22 08:31:02 PDT
Sorry, messages ports were introduced in r36891: http://trac.webkit.org/changeset/36891/trunk/WebCore/bindings/js/JSDOMWindowCustom.cpp
Comment 2 Adam Barth 2011-06-22 10:27:25 PDT
Ouch.  I wonder if we should change the spec.  Does Firefox match the spec?
Comment 3 Alexey Proskuryakov 2011-06-22 10:33:11 PDT
We certainly matched the spec at the time of adding the code.
Comment 4 Ian 'Hixie' Hickson 2011-06-22 21:35:13 PDT
If you plan to change this, hold off a few days because I'm about to update this API anyway and I wouldn't want to end up changing it around and having you waste your time.

The problem with the order (message, ports, targetOrigin) is that it means you don't know what the second argument is unless you check its type or check the number of arguments. With the order (message, targetOrigin, ports) the optional argument is the last one, not the middle one.

Does anyone else support sending ports yet? If not this might be a moot point and we might be able to change it easily. How much WebKit-specific content is there out there using this?
Comment 5 Yury Semikhatsky 2011-06-22 23:14:56 PDT
(In reply to comment #4)
> 
> Does anyone else support sending ports yet? If not this might be a moot point and we might be able to change it easily. How much WebKit-specific content is there out there using this?

Checked with Firefox 5.0 on Mac. LayoutTests/fast/events/message-port.html fails on attempt to create MessageChannel - there is no such constructor. Firefox doesn't support SharedWorkers either. So I don't see any way even to create a MessagePort in Firefox.
Comment 6 johnjbarton 2011-08-29 21:39:00 PDT
It looks like the WebKit API is
  otherWindow.postMessage(message, ports);
Specifically a third argument is not allowed. V8 at least requires the |ports| to be an Array.
Comment 7 johnjbarton 2011-08-29 21:48:09 PDT
(In reply to comment #6)
> It looks like the WebKit API is
>   otherWindow.postMessage(message, ports);
> Specifically a third argument is not allowed. V8 at least requires the |ports| to be an Array.

At least in Chrome (V8), the third argument is required and is checked as a targetOrigin. So perhaps I am not reading the WebKit source correctly.
Comment 8 Ian 'Hixie' Hickson 2011-10-17 14:43:44 PDT
Looks like WebKit actually supports the second and third arguments in either order. Is that intentional?
Comment 9 Ian 'Hixie' Hickson 2011-10-17 14:49:09 PDT
I've put that in the spec, to hopefully end this confusion about which argument is second and which is third once and for all. Looks like only WebKit implements MessagePort through postMessage() correctly enough for this to matter, and the spec now matches WebKit.
Comment 10 Ian 'Hixie' Hickson 2011-10-17 15:14:14 PDT
Actually nevermind, my testing was bogus.
Comment 11 Ian 'Hixie' Hickson 2011-10-17 15:20:32 PDT
Opera implements the spec as written here. Having the optional argument in the middle (as WebKit has it) seems generally bad, so if we're not going to support either order, I guess we should just go with what's in the spec now.

Please let me know if this causes compat issues.
Comment 12 Sam Weinig 2016-12-05 16:14:30 PST
Fixed in r209303: <http://trac.webkit.org/changeset/209303>