Bug 21419 – Add a layout test to verify popup properties while they're opening

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Pam Greene (IRC:pamg)]

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.

[Pam Greene (IRC:pamg)]

Created attachment 24131 [details]
Layout test and expected result

[Sam Weinig]

Comment on attachment 24131 [details]
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!

[Eric Seidel (no email)]

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...

[Eric Seidel (no email)]

Comment on attachment 24131 [details]
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.

[Eric Seidel (no email)]

Comment on attachment 24131 [details]
Layout test and expected result

Sam tells me notifyDone() is part of destination.html... so this is OK.

[Pam Greene (IRC:pamg)]

(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.

[Sam Weinig]

(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.

[Pam Greene (IRC:pamg)]

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.

[Sam Weinig]

(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.

[Pam Greene (IRC:pamg)]

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.)

[Sam Weinig]

(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.

[Adam Barth]

Will land.

[Adam Barth]

My Mac Mini died in the middle of testing this patch.  I'm going to take it into the store tomorrow.

[Adam Barth]

It's going to be a week before I get my Mac Mini back.  Unassigning.

[Pam Greene (IRC:pamg)]

I'll land it, now that I have cred.

[Pam Greene (IRC:pamg)]

Submitted r37595.