488313 - getDownloadsController() should not spawn a new download manager window

Status: RESOLVED WONTFIX

(Testing Graveyard :: Mozmill, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Mozmill test

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090413 Shiretoko/3.5b4pre ID:20090413031313

Given by bug 487755 comment 18 the call to getDownloadsController() spawns a new download manager window. This makes it impossible to test if the menu click or any other action was successful. The download manager appears in either way when trying to get a controller.

I've attached a testcase which shows that the test doesn't fail.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There would be a workaround to do the testing via the window mediator but having this fixed in Mozmill would make our life drastically easier. Giving a P1 priority.

Comment 2

[Mikeal Rogers [:mikeal] (mrogers@)]

All the getController methods call the underlying function below the menu for opening these different windows.

It appears that the menu function for the download manager, BrowserDownloadsUI, always opens a new window.

This should be fixed by checking for the window of a proper windowtype and if there isn't one, using this method.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mikeal, just for short. So it will not create a new download manager window when calling getDownloadsController? I hope I understand it correctly. That would be the expected behavior here.

Comment 4

[Mikeal Rogers [:mikeal] (mrogers@)]

It should only open a new one if one is not already open.

The naming convention for those methods is 

getSomeController() // Gets existing window or creates new one and returns controller.
newSomeController() // Creates new window no matter what and return controller.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mmh so it wouldn't help us here. Reading your last comment would make this bug invalid because it opens a new download manager window when none is open yet. The problem we have is that using a menu shortcut could fail in opening the downloads manager. Means we cannot use getDownloadsController() here to check if it was opened but have to use the WindowsMediator to check for its existence?

For me getSomething doesn't imply that a new window is created.

Comment 6

[Mikeal Rogers [:mikeal] (mrogers@)]

Yup, that's something you want to use the window mediator for.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #6)
> Yup, that's something you want to use the window mediator for.

Ok. Means we have to close this test as invalid.

Clint, I believe it would make sense if TestDev could help QAE here in creating a function which gets a window class/name as parameter and checks if a window of the given name is present. What do you think?

Joel, we have to wait for your test a bit.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mikeal, I did some further investigations and the following works fine:

var testDownloadManager = function(){
  var downloadController = new mozmill.controller.MozMillController(mozmill.utils.getWindowByTitle("Downloads"));
  downloadController.waitForElement(new elementslib.ID(downloadController.window.document, "searchbox"), 1000);
}

Line 1 will throw an exception if no download manager window is open. We can go this way right now. But I'm still thinking that getDownloadsController has a misleading name. Feel free to close it as invalid.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mikeal, one more question to:

http://code.google.com/p/mozmill/source/browse/trunk/mozmill/extension/resource/modules/controller.js#148

Right now we spent twice the timeout in this function if the given Window isn't existent. Couldn't we add a new parameter for a timeout? It will improve checks for a negative test. If yes, I should better file a new bug on that issue.

Comment 10

[Mikeal Rogers [:mikeal] (mrogers@)]

That isn't a timeout that is intended to wait for the window to "exist" it's suppose to wait for the window to *render*. It's in there so that tests will wait for the window to finish loading before trying to simulate user interactions on it. The MozMillController object assumes it is passed a valid and open window object and there isn't any code in the constructor to check otherwise.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, it seems that we have found a neat way to circumvent the getDownloadsController call. We can leave it as it is now. Thanks Mikeal.