Bug 24523

Summary: Add test to check that an invalid second argument of window.postMessage is ignored
Product: WebKit Reporter: Pam Greene (IRC:pamg) <pam>
Component: DOMAssignee: Pam Greene (IRC:pamg) <pam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
New test + result
none
Patch addressing Alexey's comments ap: review+

Pam Greene (IRC:pamg)
Reported 2009-03-11 15:03:58 PDT
Make sure that the second argument of window.postMessage is ignored if it's not a message port.
Attachments
New test + result (2.96 KB, patch)
2009-03-11 15:05 PDT, Pam Greene (IRC:pamg)
no flags
Patch addressing Alexey's comments (2.20 KB, patch)
2009-03-12 11:02 PDT, Pam Greene (IRC:pamg)
ap: review+
Pam Greene (IRC:pamg)
Comment 1 2009-03-11 15:05:04 PDT
Created attachment 28495 [details] New test + result
Alexey Proskuryakov
Comment 2 2009-03-12 01:09:25 PDT
Is this correct behavior, or an implementation quirk? I doubt that I fully investigated this case when writing the code - but HTML5 seems to imply that an exception should be raised. What does Firefox do? I don't think that using the test framework here helps much, as we don't have a long series of tests, and this isn't a fast/js test. As mentioned in another bug, such usage of successfullyParsed won't work in most interesting cases.
Pam Greene (IRC:pamg)
Comment 3 2009-03-12 11:02:35 PDT
Created attachment 28534 [details] Patch addressing Alexey's comments (In reply to comment #2) > Is this correct behavior, or an implementation quirk? I doubt that I fully > investigated this case when writing the code - but HTML5 seems to imply that an > exception should be raised. What does Firefox do? Firefox (Mac 3.0.7) raises an exception. The HTML4 spec doesn't say what to do. HTML5 does imply that an exception should be raised. I'd suggest adding this test as it is for now, to catch unintentional changes, and updating it when there's a deliberate switchover to HTML5 behavior. > I don't think that using the test framework here helps much, as we don't have a > long series of tests, and this isn't a fast/js test. As mentioned in another > bug, such usage of successfullyParsed won't work in most interesting cases. Yes, notice that successfullyParsed is not used in the normal way here, just inserted to make the output look like what people expect to see. In any case, I've pulled out the test framework in this patch, since as you say it's not adding much.
Alexey Proskuryakov
Comment 4 2009-03-12 11:11:27 PDT
Comment on attachment 28534 [details] Patch addressing Alexey's comments r=me - at least this tests that we don't crash! Please add a comment saying that the results are likely incorrect, so that the test doesn't scare away the person fixing the behavior.
Pam Greene (IRC:pamg)
Comment 5 2009-03-12 15:02:07 PDT
Comment added. Landed in r41651.
Note You need to log in before you can comment on or make changes to this bug.