21419 2008-10-06 17:01:14 -0700 Add a layout test to verify popup properties while they're opening 2008-10-14 15:49:57 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 pam pam abarth sam oldest_to_newest 94296 0 pam 2008-10-06 17:01:14 -0700 Different browsers return different things for the properties of a popup window while the window is opening. In particular, the behavior of window.location, window.href, and window.pathname vary. The Selenium web app test system -- http://selenium.openqa.org -- depends on this behavior. When running the attached test, popup.location reports as follows: Safari ToT: location: / Firefox: location: about:blank Chrome: location: about:blank IE7: location: file:///C:/<path to>/fast/dom/Window/resources/destination.html My goal is to contribute all of Chromium's new layout tests promptly for everyone's benefit, so even if it's decided that WebKit will change its behavior, I'd still vote for submitting the attached files as they are for now, filing a new bug, and updating the result when it changes. 94298 1 24131 pam 2008-10-06 17:06:19 -0700 Created attachment 24131 Layout test and expected result 94338 2 24131 sam 2008-10-07 00:20:31 -0700 Comment on attachment 24131 Layout test and expected result r=me. We are trying to move to more tests where the expected results are clear, so in the future, that would be a good thing to do. But this looks good. Thanks! 94339 3 eric 2008-10-07 00:53:00 -0700 What Sam meant, was that we're trying to move towards more tests where the test itself contains the expected results. You can see this in all the "shouldBe" based tests which use the scripts in fast/js/resources and the TEMPLATE.html system with make-js-test-wrappers generated test files. the fast/js tests aren't very well documented, but they're pretty simple to write. Any time you find a resources/ directory containing a TEMPLATE.html file, all of the .js files in that directory are examples of the fast/js testing system, which uses shouldBe. This test could then be re-written as something like: description("Test to make sure that popup's have no properties while opening to match other browsers https://bugs.webkit.org/show_bug.cgi?id=21419"); // dumpAsText is handled for you in fast/js style tests if (window.layoutTestController) { layoutTestController.setCanOpenWindows(); layoutTestController.waitUntilDone(); } var popup = window.open('resources/destination.html'); shouldBe("popup.location", "/"); shouldBe("popup.location['href']", "/"); shouldBe("popup.location['pathname']", "/"); shouldBe("popup.location['hash']", ""); shouldBe("popup.location['port']", ""); shouldBe("popup.location['protocol']", ""); shouldBe("popup.location['search']", ""); shouldBe("popup.location['pathname']", ""); Wait... actually this test never calls "notifyDone()" so it will just hit the timeout every time! That's bad. :( I have to change review to r-, unless I missed something... 94340 4 24131 eric 2008-10-07 00:54:07 -0700 Comment on attachment 24131 Layout test and expected result Yeah, this needs to call notifyDone() or not call waitUntilDone(), otherwise this test just takes longer than necessary to run. Consider looking at the new fancy fast/js testing system if you have interest. 94341 5 24131 eric 2008-10-07 00:56:34 -0700 Comment on attachment 24131 Layout test and expected result Sam tells me notifyDone() is part of destination.html... so this is OK. 94396 6 pam 2008-10-07 11:27:10 -0700 (In reply to comment #3) > What Sam meant, was that we're trying to move towards more tests where the test > itself contains the expected results. I'd agree in general, but that does cause problems for tests like this one where different embedders may legitimately want different results. 94414 7 sam 2008-10-07 12:44:49 -0700 (In reply to comment #6) > I'd agree in general, but that does cause problems for tests like this one > where different embedders may legitimately want different results. I can see no reason why different ports would want different results here. 94415 8 pam 2008-10-07 12:49:15 -0700 Well, I know Chrome reports "location: about:blank" so Selenium works. I don't know if there's a specific reason that Safari reports "location: /" instead, or if it's just that nobody had a reason to care before. 94416 9 sam 2008-10-07 12:54:16 -0700 (In reply to comment #8) > Well, I know Chrome reports "location: about:blank" so Selenium works. I don't > know if there's a specific reason that Safari reports "location: /" instead, or > if it's just that nobody had a reason to care before. > Presumably it is a bug. 94419 10 pam 2008-10-07 13:05:21 -0700 Is it acceptable to submit this test and expected result as they are, and file a separate bug to fix it? Or should I morph this report and keep the test pending until the behavior is fixed? (I'd selfishly prefer the former, but I recognize that the latter would be better practice.) 94425 11 sam 2008-10-07 13:23:57 -0700 (In reply to comment #10) > Is it acceptable to submit this test and expected result as they are, and file > a separate bug to fix it? Or should I morph this report and keep the test > pending until the behavior is fixed? (I'd selfishly prefer the former, but I > recognize that the latter would be better practice.) Oh, no, please land as is and file a follow up bug. The main point of tests is to test for changes in behavior, so having them in the trees is the most important thing. My point was only that any new tests would be preferred in the other format, to solve a dual purpose. 95253 12 abarth 2008-10-14 01:16:00 -0700 Will land. 95274 13 abarth 2008-10-14 03:01:20 -0700 My Mac Mini died in the middle of testing this patch. I'm going to take it into the store tomorrow. 95334 14 abarth 2008-10-14 12:28:06 -0700 It's going to be a week before I get my Mac Mini back. Unassigning. 95338 15 pam 2008-10-14 12:37:48 -0700 I'll land it, now that I have cred. 95368 16 pam 2008-10-14 15:49:57 -0700 Submitted r37595. 24131 2008-10-06 17:06:19 -0700 2008-10-07 00:56:34 -0700 Layout test and expected result window-url.txt text/plain 1657 pam SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDM3MzUz KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTAgQEAKKzIwMDgtMTAt MDYgIFBhbWVsYSBHcmVlbmUgIDxwYW1AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogZmFzdC9kb20vV2luZG93L3dpbmRvdy1v cGVuLXBlbmRpbmctdXJsLWV4cGVjdGVkLnR4dDogQWRkZWQuCisgICAgICAgICogZmFzdC9kb20v V2luZG93L3dpbmRvdy1vcGVuLXBlbmRpbmctdXJsLmh0bWw6IEFkZGVkLgorCiAyMDA4LTEwLTA0 ICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBE YXJpbiBBZGxlci4KSW5kZXg6IGZhc3QvZG9tL1dpbmRvdy93aW5kb3ctb3Blbi1wZW5kaW5nLXVy bC1leHBlY3RlZC50eHQKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gZmFzdC9kb20vV2luZG93L3dpbmRvdy1vcGVu LXBlbmRpbmctdXJsLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKKysrIGZhc3QvZG9tL1dpbmRv dy93aW5kb3ctb3Blbi1wZW5kaW5nLXVybC1leHBlY3RlZC50eHQJKHJldmlzaW9uIDApCkBAIC0w LDAgKzEsMTAgQEAKK2xvY2F0aW9uOiAvCitocmVmOiAvCitwYXRobmFtZTogLworaG9zdDogCito b3N0bmFtZTogCitoYXNoOiAKK3BvcnQ6IAorcHJvdG9jb2w6IDoKK3NlYXJjaDogCisKSW5kZXg6 IGZhc3QvZG9tL1dpbmRvdy93aW5kb3ctb3Blbi1wZW5kaW5nLXVybC5odG1sCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIGZhc3QvZG9tL1dpbmRvdy93aW5kb3ctb3Blbi1wZW5kaW5nLXVybC5odG1sCShyZXZpc2lv biAwKQorKysgZmFzdC9kb20vV2luZG93L3dpbmRvdy1vcGVuLXBlbmRpbmctdXJsLmh0bWwJKHJl dmlzaW9uIDApCkBAIC0wLDAgKzEsMTYgQEAKKzxzY3JpcHQ+CitpZiAod2luZG93LmxheW91dFRl c3RDb250cm9sbGVyKSB7CisgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIuZHVtcEFzVGV4dCgpOwor ICAgIGxheW91dFRlc3RDb250cm9sbGVyLnNldENhbk9wZW5XaW5kb3dzKCk7CisgICAgbGF5b3V0 VGVzdENvbnRyb2xsZXIud2FpdFVudGlsRG9uZSgpOworfQorCit2YXIgcG9wdXAgPSB3aW5kb3cu b3BlbigncmVzb3VyY2VzL2Rlc3RpbmF0aW9uLmh0bWwnKTsKK2RvY3VtZW50LndyaXRlKCdsb2Nh dGlvbjogJyArIHBvcHVwLmxvY2F0aW9uICsgJzxici8+Jyk7Cit2YXIgcHJvcHMgPSBbJ2hyZWYn LCAncGF0aG5hbWUnLCAnaG9zdCcsICdob3N0bmFtZScsICdoYXNoJywgJ3BvcnQnLAorICAgICAg ICAgICAgICdwcm90b2NvbCcsICdzZWFyY2gnXTsKK2ZvciAodmFyIGkgPSAwOyBpIDwgcHJvcHMu bGVuZ3RoOyArK2kpIHsKKyAgdmFyIHByb3AgPSBwcm9wc1tpXTsKKyAgZG9jdW1lbnQud3JpdGUo cHJvcCArICc6ICcgKyBwb3B1cC5sb2NhdGlvbltwcm9wXSArICc8YnIvPicpOworfQorPC9zY3Jp cHQ+Cg==