518386 - Helper function to handle non-modal windows

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

When we handle multiple windows in parallel we have a lot of code to write. It would be great to have a helper function which covers opening/closing of independent windows. Further we need an observer to reliable check if the wanted window has really opened. A good example can be found in attachment 336629 [details] [diff] [review].

I will get to this next quarter ASAP.

Comment 1

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

We would have to go the way as how it is used in this pb mochitest: attachment 413211 [details] [diff] [review].

Comment 2

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

We should use this function as a wrapper so we can handle those windows a similar way like modal dialogs. That way we can catch exceptions and make sure that the non-modal window is closed properly.

Comment 3

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

Attachment: Window helper and massive test updates

That's probably one of our most important patches those days. I hope that we can make mostly all of our orange failures disappear. With this patch we have a clean way to handle non-modal windows which also get closed when a test inside that window fails. At the moment those windows stay open and block key events to get entered in text boxes.

This is a function I would like to see in Mozmill core, hopefully already for Mozmill 1.4.2.

Comment 4

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

Clint and Jeff, can you both please also check the patch? That would be really appreciated.

Comment 5

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

Attachment: Window helper and test updates

I did a minor failure in the helper function to set donClose to true if it hasn't been specified. I saw it right now in the diff on Bugzilla. That would be counter productive to my latest comments. This patch fixes it.

Comment 6

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

Clint, I still have to find a way to wait until the content of the window has been loaded. I wasn't really successful so far. Can you point me to some code I would have to use? Something related to DOMContentLoaded or is there a better way doing it?

Comment 7

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

Comment on attachment 450890 [details] [diff] [review]
Window helper and test updates

Dropping additional review requests for now, because those are only api changes. I will request for review on a follow-up patch with a better window loaded detection.

Comment 8

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

Landed on default and mozilla1.9.2:
http://hg.mozilla.org/qa/mozmill-tests/rev/9d908275525e
http://hg.mozilla.org/qa/mozmill-tests/rev/0f0bb401ae02

I will come up with a patch for 1.9.1 shortly.

Comment 9

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

Attachment: Window helper and test updates (1.9.1)

For mozilla1.9.1 a lot of hunks could not be applied. This is a mostly rewrite of the default patch.

Comment 10

[u279076]

Comment on attachment 451256 [details] [diff] [review]
Window helper and test updates (1.9.1)

>+  // The Disable Cookies button doesn't receive focus that fast. Means a click will
>+  // fail if sent too early. There is no property we can check so far. So lets
>+  // use a sleep call for now.
>   var acceptCookiesPref = new elementslib.ID(controller.window.document, "acceptCookies");
>+  controller.sleep(500);
>   controller.check(acceptCookiesPref, false);
> 
I'm wondering now if waitForElement does not work? There are a few other places we use sleep in this patch under similar conditions. I'd rather use that than an explicit sleep call if we can.

Other than that, this all looks good.  I'm going to r+ this patch but I think we need to do some serious consideration and investigation of alternatives to using sleep in the near future.

Comment 11

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

(In reply to comment #10)
> >+  // use a sleep call for now.
> >   var acceptCookiesPref = new elementslib.ID(controller.window.document, "acceptCookies");
> >+  controller.sleep(500);
> >   controller.check(acceptCookiesPref, false);
> > 
> I'm wondering now if waitForElement does not work? There are a few other places
> we use sleep in this patch under similar conditions. I'd rather use that than
> an explicit sleep call if we can.

This checkbox is always present, same as the show cookies button. But somehow they don't react on clicks right after selecting an item from the top dropdown box. I will see if I can find a workaround or if this is even a bug in the preferences dialog.

Comment 12

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

Landed on 1.9.1. I will create a follow-up for the investigation to remove the sleep call while waiting for the window loading to finish.

Comment 13

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

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.