1390431 - TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_window_open_link_opening_behaviour
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_external_link_opening_behaviour
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_return_to_provisioner_on_error_XML
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_disabled_fields_when_searching
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_search_button_disabled_if_no_lang_support
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_provider_language_wildcard
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_search_button_disabled_if_no_query_on_init
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_get_new_account_focuses_existing_ap_tab
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_per_address_prices

First seen Tue Aug 15, 2017, 0:04:04
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=36bd1eaceefb0e11a64ac72870faedabd3a88315

Log says:
https://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1502748266/comm-central_win7_ix_test-mozmill-bm110-tests1-windows-build11.txt.gz

INFO -  SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_window_open_link_opening_behaviour
INFO -    EXCEPTION: Timeout waiting for the content tab to open
INFO -      at: utils.js line 409
INFO -         TimeoutError utils.js:409 13
INFO -         waitFor utils.js:447 11
INFO -         open_content_tab_with_click test-content-tab-helpers.js:202 3
INFO -         test_window_open_link_opening_behaviour test-newmailaccount.js:960 3
INFO -         Runner.prototype.wrapper frame.js:585 9
INFO -         Runner.prototype._runTestModule frame.js:655 9
INFO -         Runner.prototype.runTestModule frame.js:701 3
INFO -         Runner.prototype.runTestDirectory frame.js:525 7
INFO -         runTestDirectory frame.js:707 3
INFO -         Bridge.prototype._execFunction server.js:179 10
INFO -         Bridge.prototype.execFunction server.js:183 16
INFO -         Session.prototype.receive server.js:282 3
INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3
INFO -  SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_external_link_opening_behaviour
INFO -    EXCEPTION: Timeout waiting for modal dialog to open.
INFO -      at: utils.js line 409
INFO -         TimeoutError utils.js:409 13
INFO -         waitFor utils.js:447 11
INFO -         WindowWatcher_waitForModalDialog test-window-helpers.js:376 5
INFO -         wait_for_modal_dialog test-window-helpers.js:613 3
INFO -         get_to_order_form test-newmailaccount.js:876 3
INFO -         test_external_link_opening_behaviour test-newmailaccount.js:974 3
INFO -         Runner.prototype.wrapper frame.js:585 9
INFO -         Runner.prototype._runTestModule frame.js:655 9
INFO -         Runner.prototype.runTestModule frame.js:701 3
INFO -         Runner.prototype.runTestDirectory frame.js:525 7
INFO -         runTestDirectory frame.js:707 3
INFO -         Bridge.prototype._execFunction server.js:179 10
INFO -         Bridge.prototype.execFunction server.js:183 16
INFO -         Session.prototype.receive server.js:282 3

and more of the same.

M-C last good: df9beb781895fcd0493c21e95ad313e004
M-C first bad: 824d4f269c6323e1ad2bd8ebeb6496d60b

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=df9beb781895fcd0493c21e95ad313e004&tochange=824d4f269c6323e1ad2bd8ebeb6496d60b

Looks like bug 1377580.

A little analysis:

open_content_tab_with_click has:
function open_content_tab_with_click(aElem, aExpectedURL, aController) {
  if (aController === undefined)
    aController = mc;

  let preCount = aController.tabmail.tabContainer.childNodes.length;
  aController.click(new elib.Elem(aElem));
  utils.waitFor(() => (  <=== line 202 were we hit a timeout.

Equally:

function open_provisioner_window() {
  mc.click(new elib.Elem(mc.menus.menu_File.menu_New.newCreateEmailAccountMenuItem));
}
which is called at test-newmailaccount.js:875 just before timing out at test-newmailaccount.js:876.

So all these failures have the same common cause that mc.click() doesn't see to work any more.

I guess the actual function still works, just not the test.

Olli, any insight?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Hmm, Thunderbird does have its own nsIBrowserDOMWindow implementation, https://dxr.mozilla.org/comm-central/rev/20658ec6d032a2dff9c281d6431e9004d5a33968/mail/base/content/mailWindow.js#650 at least.
That should be updated the same way https://bug1377580.bmoattachments.org/attachment.cgi?id=8895362 updated browser/base/content/browser.js

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1390431-nsIBrowserDOMWindow-createContentWindow.patch

Thanks Olli, with the straight port from
https://hg.mozilla.org/mozilla-central/rev/216dc8344ce0#l1.12
our test passes now.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1390431-nsIBrowserDOMWindow-createContentWindow.patch (v2)

There seems to be a little confusion as to what the parameters should be. I copied the M-C changeset, but that appears to be wrong, see bug 1377580 comment #27.

So here's an improved version.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1390431-nsIBrowserDOMWindow-createContentWindow.patch (v2b)

Sorry about the noise, shortened line a little.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: 1390431-nsIBrowserDOMWindow-createContentWindow-sm.patch (v1)

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #3)
> There seems to be a little confusion as to what the parameters should be.
M-C are correcting this: attachment 8897387 [details] [diff] [review]. So the patches here are correctly matching M-C only that we tolerate a missing last argument.

Comment 7

[Eugen Sawin [:esawin]]

I wasn't aware of Thunderbird's nsIBrowserDOMWindow implementation. Sorry for causing the confusion and thanks for the prompt resolution.

Comment 8

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8897384 [details] [diff] [review]
1390431-nsIBrowserDOMWindow-createContentWindow-sm.patch (v1)

Thanks. Looks good. SeaMonkey needs some other changes here too. I am working on it in bug 1379369 but got sidetracked. Will test it when I come around to fix the other bug.

One question. Not 100% sure how js ticks here. Shouldn't it work to move the aTriggeringPrincipal = null to getContentWindowOrOpenURI and just pass the parameter in the other two?

Comment 9

[Jorg K (CEST = GMT+2)]

Well, as we have discussed on various occasions (bug 1316389, bug 1379386, bug 1321013), we couldn't find a caller for openURI. Looks like the failing test is also not calling it, but rather createContentWindow() which was missing.

However, we do know that according to nsIBrowserDOMWindow.idl, openURI does have the principal parameter.

So we're implementing openURI() and createContentWindow() according to the spec in the IDL and since we don't know who calls them, we'd better cater for missing principal parameters.

Moving the aTriggeringPrincipal = null to getContentWindowOrOpenURI would most likely also work, but it's not necessary since we're controlling calls to that function, but we're not controlling calls to the other two. I'd sleep better with the patches I proposed ;-)

Comment 10

[:aceman]

Comment on attachment 8897383 [details] [diff] [review]
1390431-nsIBrowserDOMWindow-createContentWindow.patch (v2b)

Review of attachment 8897383 [details] [diff] [review]:
-----------------------------------------------------------------

Why do we need to reimplement these functions when the code is the same as in m-c? Isn't there some inheritance mechanism that could be used?

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1390431-nsIBrowserDOMWindow-createContentWindow.patch (v2c).

Since we were always wondering whether and where openURI is called or whether it could even be removed, I've added a comment to document the solution of this mystery ;-)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/90afaded37d5
Port bug 1377580 to TB: Provide nsIBrowserDOMWindow.createContentWindow. r=aceman
https://hg.mozilla.org/comm-central/rev/08c99aaee2b8
Port bug 1377580 to SM: Provide nsIBrowserDOMWindow.createContentWindow. r=frg