Closed Bug 518386 Opened 11 years ago Closed 10 years ago

Helper function to handle non-modal windows

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

(Whiteboard: [lib])

Attachments

(2 files, 1 obsolete file)

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.
We would have to go the way as how it is used in this pb mochitest: attachment 413211 [details] [diff] [review].
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.
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
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.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #450889 - Flags: review?(anthony.s.hughes)
Clint and Jeff, can you both please also check the patch? That would be really appreciated.
Whiteboard: [mozmill-1.4.2?]
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.
Attachment #450889 - Attachment is obsolete: true
Attachment #450890 - Flags: review?(anthony.s.hughes)
Attachment #450889 - Flags: review?(jhammel)
Attachment #450889 - Flags: review?(ctalbert)
Attachment #450889 - Flags: review?(anthony.s.hughes)
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?
Attachment #450890 - Flags: review?(anthony.s.hughes) → review+
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.
Attachment #450890 - Flags: review?(jhammel)
Attachment #450890 - Flags: review?(ctalbert)
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.
Whiteboard: [mozmill-1.4.2?] → [mozmill.next?]
For mozilla1.9.1 a lot of hunks could not be applied. This is a mostly rewrite of the default patch.
Attachment #451256 - Flags: review?(anthony.s.hughes)
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.
Attachment #451256 - Flags: review?(anthony.s.hughes) → review+
(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.
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill.next?] → [mozmill.next?][mozmill-doc-needed]
Summary: [shared module] Helper function to handle non-modal windows → Helper function to handle non-modal windows
Whiteboard: [mozmill.next?][mozmill-doc-needed] → [shared module][mozmill-doc-needed][mozmill-2.0?]
Component: Mozmill → Mozmill Tests
QA Contact: mozmill → mozmilltests
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.
Product: Testing → Mozilla QA
Version: Trunk → unspecified
Component: Mozmill Tests → Mozmill Shared Modules
Component: Mozmill Shared Modules → Mozmill Tests
Whiteboard: [shared module][mozmill-doc-needed][mozmill-2.0?] → [lib]
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.