Bug 18550 - cross-frame-access-call.html doesn't really test window.open(), since popups are blocked
Summary: cross-frame-access-call.html doesn't really test window.open(), since popups ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-17 12:34 PDT by Eric Roman
Modified: 2008-06-08 12:09 PDT (History)
0 users

See Also:


Attachments
add layoutTestController.setCanOpenWindows(true) (663 bytes, patch)
2008-04-17 13:32 PDT, Eric Roman
aroben: review-
Details | Formatted Diff | Diff
add layoutTestController.setCanOpenWindows(true) (2.22 KB, patch)
2008-04-18 12:31 PDT, Eric Roman
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Roman 2008-04-17 12:34:02 PDT
http/tests/security/cross-frame-access-call.html
contains a test for cross domain violation on "window.open.call()", by checking for a return value of undefined.

However since popups are disabled, window.open will return undefined regardless.
Comment 1 Eric Roman 2008-04-17 13:32:50 PDT
Created attachment 20636 [details]
add layoutTestController.setCanOpenWindows(true)
Comment 2 Adam Roben (:aroben) 2008-04-18 07:01:22 PDT
Comment on attachment 20636 [details]
add layoutTestController.setCanOpenWindows(true)

Thanks for the patch!

You should leave the "requestee" field blank in most cases when putting a patch up for review.

You'll need to create a ChangeLog entry. See <http://webkit.org/coding/contributing.html> for information on how to prepare and submit a patch.

Have you verified that the test still passes?

r- so that a ChangeLog can be added.
Comment 3 Eric Roman 2008-04-18 12:31:45 PDT
Created attachment 20673 [details]
add layoutTestController.setCanOpenWindows(true)

Sorry about that aroben! Fixed, new patch uploaded.

I ran the layout test, and it passes. I did have to change a line number in the expected output though, since this patch added two lines.
Comment 4 Adam Roben (:aroben) 2008-04-27 22:27:21 PDT
Comment on attachment 20673 [details]
add layoutTestController.setCanOpenWindows(true)

+2008-04-18  Eric Roman  <minatoar@gmail.com>
+
+        Reviewed by NOBODY (OOPS!).
+
+        * http/tests/security/cross-frame-access-call-expected.txt: Increase a console line number (since added two lines)
+        * http/tests/security/cross-frame-access-call.html: Enable popups for this layout test. The test checks that window.open is returns undefined (same-domain enforcement), however unless popups are enabled it returns undefined regardless.
+

It's customary to list the bug title and URL of the bug you're fixing in your ChangeLog entry (though I think unfortunately this is not mentioned on our website anywhere). Whomever lands this patch can add that for you.

r=me. Thanks!
Comment 5 Darin Adler 2008-06-08 12:09:52 PDT
Committed revision 34447.