We need to add a gaia-ui-test modal dialog test, to test the code paths taken in Bug 779284. We should be able to use something like mochitest's test container to create our own test app and trigger a modal from there.

This sounds like an ideal unit tests for gaiatest. Malini: Do you have some more details regarding your suggestions for creating our own test app? It would be great to be able to do this for some of the other unit tests to make them independent from the packaged apps.

(In reply to Dave Hunt (:davehunt) from comment #1) > This sounds like an ideal unit tests for gaiatest. Malini: Do you have some > more details regarding your suggestions for creating our own test app? It > would be great to be able to do this for some of the other unit tests to > make them independent from the packaged apps. Not yet, I'm going to be playing around with the idea today. Jgriffin mentioned this approach is used by mochitest, so he'd have more information, but I hope to have a working test by today.

Comment on attachment 793209 [details] Pointer to pull request I'd like to have Dave take a look too, to see what you think of this approach. My concern with using Mochitest's test-container is that it uses the mozapp attribute to set up its permissions with a mochitest server url, so the default test-container without Mochitest running won't have the permissions other tests may want to use (permissions to let you load another app, for example). Not sure if this is something we want to provide or not for other tests. If so, we can add another test-container with its permissions manifest somewhere accessible.

Comment on attachment 793209 [details] Pointer to pull request I've added a few comments to the pull request, but the approach looks great to me. I'm not sure I fully understand the limitations as explained in comment 4 though.

(In reply to Dave Hunt (:davehunt) from comment #5) > Comment on attachment 793209 [details] > Pointer to pull request > > I've added a few comments to the pull request, but the approach looks great > to me. I'm not sure I fully understand the limitations as explained in > comment 4 though. Say you want to use the iframe in test-container to load an app. You need special permissions to do so. In the current state, you will be prevented from loading any app:// urls. I don't really see why you'd want to do that, since the test-container should be used like an app sandbox (a place where you simulate an app and not load an app itself).

Comment on attachment 793209 [details] Pointer to pull request I've updated the PR as per comments (same url), and tested it against b2g desktop and device.

Comment on attachment 793209 [details] Pointer to pull request As commented in the PR, this is passing on desktop for me, but failing on device. :( Also, let's add a TBPL manifest entry too.

(In reply to Dave Hunt (:davehunt) from comment #8) > Comment on attachment 793209 [details] > Pointer to pull request > > As commented in the PR, this is passing on desktop for me, but failing on > device. :( Also, let's add a TBPL manifest entry too. What build did you test against? It *just* landed in b2g18 yesterday. I tested on unagi using today's mozilla-central and b2g18 eng builds from pvt, and the test passes on both of them. I've updated the PR with the manifest changes.

Comment on attachment 793209 [details] Pointer to pull request The test in general looks ok to me; I'll let you and davehunt work out why it isn't working on dave's inari.

With bug 779284 resolved again, this is clear to go ahead. Make sure you land in gaia now though, rather than the gaia-ui-tests repository.

this has probably rotted to feed cyber plants. Closing this since modal support for desktop has landed and has a number of tests.

(In reply to David Burns :automatedtester from comment #12) > this has probably rotted to feed cyber plants. Closing this since modal > support for desktop has landed and has a number of tests. We haven't landed modal dialog support for Desktop yet. What has been landed is the handling of tab modal dialogs. So not sure if that is what this bug is about. For more info see bug 887274.