723910 - Account Provisioner fails to close order form tab and report success

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

The following test failures have appeared on trunk:

TEST-START | /buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/newmailaccount/test-newmailaccount.js | test_get_an_account
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "controller.click()"}
Test Failure: Timeout waiting for modal dialog to open.
TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/newmailaccount/test-newmailaccount.js | test-newmailaccount.js::test_get_an_account


TEST-START | /buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/newmailaccount/test-newmailaccount.js | test_throws_console_error_on_corrupt_XML
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.waitFor()"}
2012-02-02 15:11:09	mail.provider	ERROR	Got a result back for a provider that the user did not select: bar
2012-02-02 15:11:09	mail.provider	ERROR	Got a result back for a provider that the user did not select: foo
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "controller.click()"}
2012-02-02 15:11:15	mail.provider	ERROR	Something went wrong loading the provider list JSON file. Going into offline mode.
Test Failure: Timed out waiting for console message: Problem interpreting provider XML:
TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/newmailaccount/test-newmailaccount.js | test-newmailaccount.js::test_throws_console_error_on_corrupt_XML


This is likely due to something landing on mozilla-central.  The following commits are within the regression timeline:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e18c7bc2c28e&tochange=c538da196e34

Comment 1

[Mike Conley (:mconley) (:⚙️)]

After some poking about, I'm pretty sure the problem was introduced with http://hg.mozilla.org/mozilla-central/rev/4d8f729aa1aa

And this isn't just broken tests - this has actually broken account provisioner.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

I'm getting this in the Error Console when manually running the provisioner:

Timestamp: 12-02-03 09:21:59 AM
Error: Problem interpreting provider XML:TypeError: attempt to run compile-and-go script on a cleared scope
Source File: chrome://messenger/content/newmailaccount/uriListener.js
Line: 217

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Steps to reproduce:

1)  File > New > Get a New Mail Account
2)  Enter a name into the dialog, and perform search with Hover.com
3)  Choose a found address for purchase.
4)  In the content-tab that opens, wait for the provisioner window to clear, fill in the required fields with bogus information.

For the credit card #, use: 4111-1111-1111-1111.  The 3-digit security code can be anything.

5)  Press submit

Expected results:

The account should be created in TB, the order form tab should close, and a window should appear to let us know that we've successfully created an account.

Actual results:

The 3pane tab is focused, but the order form content tab is not closed.  No account is created, and we seem to fail silently.  The following error is in the Error Console.

Timestamp: 12-02-03 09:21:59 AM
Error: Problem interpreting provider XML:TypeError: attempt to run compile-and-go script on a cleared scope
Source File: chrome://messenger/content/newmailaccount/uriListener.js
Line: 217

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Two amendments to the above STR:

1)  The credit card # should be 4111111111111111 (so, the same as above, without the dashes)
2)  It appears that the account *is* being created - we just aren't spawning the success window.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I've narrowed the problem down to the tabMonitor that we register to listen for the closing of the order form tab:

http://mxr.mozilla.org/comm-central/source/mail/components/newmailaccount/content/accountProvisioner.js#498

When tabmail attempts to run functions on this tabMonitor, this is when we're getting the "attempt to run compile-and-go script on a cleared scope" issue.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Backing out the change from bug 680456 seems to resolve the issue.

So, with the new reality of bug 680456 landing, what is the best way to keep that tabMonitor working? Or have we uncovered a script bug?

Comment 7

[Mike Conley (:mconley) (:⚙️)]

So I spoke to dmandelin over IRC, and this isn't a script bug - what's happening is that since the Provisioner modal window is closing, all objects/scripts for which the modal window is the global get cleared.

So, to sum, when the modal dialog closes, we lose the tabMonitor we registered, and then tabmail.xml freaks out.

According to dmandelin, in order to get around this bug for now, we have to make sure that the tabMonitor (and any functions that the tab or its observers will need to fire) are created either:

a)  In a scope "higher" than the Provisioner modal
b)  In the opened tab itself

I'm not quite sure how best to do this yet.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Here's my first run at a fix.  All newmailaccount tests pass, and a manual run-through with Hover.com seems alright.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 594275 [details] [diff] [review]
Patch v1

Mark - do you have the cycles to review this?

I'm mostly just concerned that my placement of <script src="...accountProvisionerTab.js"/> in messenger.xul is incorrect - especially since it depends on being loaded *after* specialTabs.js.  Perhaps we should just put the accountProvisionerTab.js stuff in specialTabs.js?

Another thing, is that this workaround we're using doesn't have to be permanent.  According to dmandelin, if/when bug 637099 lands, our old code should work correctly.  But he said don't count on that landing for a little while.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 594275 [details] [diff] [review]
Patch v1

Jim:

You've got some experience with tabs too - got any feedback on this stuff?

-Mike

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Try build kicked off here:  http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=95f04ebf8cf4

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 594275 [details] [diff] [review]
Patch v1

I think this looks fine. If you're concerned about the placement of the accountProvisionerTab.js inclusion, I think I'd just add a comment about what it depends on.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2 (r+'d by Standard8)

Thanks Mark - I've added a quick comment in messenger.xul.  Tests seem to pass on try, so I'll be pushing.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Pushed to comm-central as http://hg.mozilla.org/comm-central/rev/50d86f477f0a

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 594384 [details] [diff] [review]
Patch v2 (r+'d by Standard8)

[Triage Comment]
Taking to comm-aurora as the bug that caused this has been fixed there as well.

Comment 16

[Mark Banner (:standard8)]

Landed: http://hg.mozilla.org/releases/comm-aurora/rev/6b006b33f661