TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js and test-retest-config.js and test-instrument-setup.js

RESOLVED FIXED in Thunderbird 50.0

Status

Thunderbird
Account Manager
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

({regression})

Trunk
Thunderbird 50.0
regression

Thunderbird Tracking Flags

(thunderbird49 unaffected, thunderbird50 affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
First seen here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=d6147076fb29cb4162ea9343658f19e5004f6508

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_mail_account_setup

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_bad_password_uses_old_settings

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-retest-config.js | test-retest-config.js::test_re_test_config

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::test_mail_account_setup

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::teardownModule

Although these failures have associated bugs, there seems to be a systemic problem that these all started going perma-orange at the same time.

mozmake SOLO_TEST=account/test-mail-account-setup-wizard.js mozmill-one
mozmake SOLO_TEST=account/test-retest-config.js mozmill-one
mozmake SOLO_TEST=instrumentation/test-instrument-setup.js mozmill-one
fail locally. They all hang when looking up the e-mail provider.

Regression windows M-C:
Last good: moz:14c5bf11d37b
First bad: moz:b9f4f3806395
(Assignee)

Comment 1

a year ago
In a local build I tried to set up an account for huhu@gmail.com and it fails to find the details. Usually Gmail is well known.

Comment 2

a year ago
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #0)
> Although these failures have associated bugs, there seems to be a systemic
> problem that these all started going perma-orange at the same time.

Yeah, this doesn't look like the existing intermittent bugs.

Comment 3

a year ago
Looks like at least some of these (I didn't check them all) fail locally too. (make SOLO_TEST=... mozmill-one)
(Assignee)

Comment 4

a year ago
(In reply to aleth [:aleth] from comment #3)
> Looks like at least some of these fail locally
All fail locally as per comment #0. Also note comment #1 that the account creation function seems to be broken which is reflected in the test failures.
(Assignee)

Comment 5

a year ago
OK, when I try to create a local account as per comment #1 I get this on the debug console:

-- Exception object --
+ _message (string) ''
+ stack (string) 465 chars
+ code (number) 0
+ uri (string) 'http://autoconfig.gmail.com/mail/config-v1.1.xml'
+ constructor (function) 6 lines
+ message (string) ''
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onerror@chrome://messenger/content/accountcreation/fetchhttp.js:117:36
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1
Bad response content
-- Exception object --
+ _message (string) 'Bad response content'
+ stack (string) 464 chars
+ code (number) -4
+ uri (string) 'https://live.mozillamessaging.com/autoconfig/v1.1/gmail.com'
+ constructor (function) 6 lines
+ message (string) 'Bad response content'
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onload@chrome://messenger/content/accountcreation/fetchhttp.js:116:35
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1
Bad response content
-- Exception object --
+ _message (string) 'Bad response content'
+ stack (string) 464 chars
+ code (number) -4
+ uri (string) 'https://live.mozillamessaging.com/autoconfig/v1.1/google.com'
+ constructor (function) 6 lines
+ message (string) 'Bad response content'
+ toString (function) 4 lines
*
-- Stack Trace --
Exception@chrome://messenger/content/accountcreation/util.js:137:5
ServerException@chrome://messenger/content/accountcreation/fetchhttp.js:262:3
FetchHTTP.prototype._response@chrome://messenger/content/accountcreation/fetchhttp.js:190:19
FetchHTTP.prototype.start/request.onload@chrome://messenger/content/accountcreation/fetchhttp.js:116:35
MsgAccountManager@chrome://messenger/content/accountUtils.js:255:9
oncommand@chrome://messenger/content/messenger.xul:1:1
(Assignee)

Comment 6

a year ago
Alice, can you locate the regression?
It seems that account creation in TB can't lookup the ISP database any more so some tests fail and it also doesn't work manually.

https://treeherder.mozilla.org/#/jobs?repo=comm-central
Last good: Wed Jun 15, 14:02:26 - rsx11m.pub@gmail.com
First bad: Wed Jun 15, 22:12:29 - stefanh@inbox.com
There you need at the nightlies that failed the tests. Clicking on the job details, you get the revisions of M-C.

Regression windows M-C:
Last good: moz:14c5bf11d37b
First bad: moz:b9f4f3806395

I've been looking at the pushlog but I can't see anything obvious.

Thanks in advance for your help.
Flags: needinfo?(alice0775)
Keywords: regression, regressionwindow-wanted

Comment 7

a year ago
Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=14c5bf11d37b9e92d27f7089d9392de2ac339bb3&tochange=b9f4f38063951cd5a8b249911aea61869f40fd1f
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=abd1ef9761da&tochange=d6147076fb29

Via local build
Last Good : c-c d6147076fb29, m-c 4b35974b5b39
First Bad : c-c d6147076fb29, m-c 87aee9d5e04d

Regressed by: 87aee9d5e04d	Thomas Wisniewski — Bug 918733 - have overrideMimeType throw INVALID_STATE_ERR if the XHR is in the DONE or LOADING states. r=sicking
Blocks: 918733
Flags: needinfo?(alice0775)

Updated

a year ago
status-thunderbird49: --- → unaffected
status-thunderbird50: --- → affected
Keywords: regressionwindow-wanted

Updated

a year ago
Component: General → Account Manager
(Assignee)

Comment 8

a year ago
Thomas and Jonas: Your change in bug 918733 broke the Thunderbird account manager. I don't know whether you're familiar with Thunderbird. We have an account manager, which, after the user enters their e-mail address, accesses the configuration database at https://live.mozillamessaging.com/autoconfig/v1.1/ to look up details about the servers for this e-mail address. For Gmail it would for example retrieve smtp.gmail.com as outgoing server. You can see error messages in comment #5.

Our code is in
https://dxr.mozilla.org/comm-central/source/mailnews/base/prefs/content/accountcreation/fetchhttp.js

Can you please advise us which changes we need to make to get our software working again or at least how to debug it.

I read in bug 918733 comment #1 related to an earlier patch:
  Note that Chrome now throws whether also specifying a charset or not.

Does that mean that it is meant not to work at all any more? Since Thunderbird can be viewed as one big Firefox add-on, would your change not also affect other add-ons?
Flags: needinfo?(wisniewskit)
Flags: needinfo?(jonas)
(Assignee)

Comment 9

a year ago
We call overrideMimeType() here:
https://dxr.mozilla.org/comm-central/source/mailnews/base/prefs/content/accountcreation/fetchhttp.js#141

Comment 10

a year ago
Your call to overrideMimeType isn't actually doing anything, since it's too late for it to have any impact once the document is loading/done. As MDN states, you have to call overrideMimeType before send(). It was a no-op before, now we're just throwing an exception rather than silently failing (which is what the WhatWG specification requires).

Unless a lot of unmaintained addons start breaking, I'd advise keeping the change. It's good to inform users when they're using overrideMimeType incorrectly, whether they're writing addons or not (especially since other browsers will throw an exception here).

Updated

a year ago
Flags: needinfo?(wisniewskit)
(Assignee)

Comment 11

a year ago
I commented out our
  this._request.overrideMimeType("text/xml");
and that does change the behaviour (it hangs completely now) which is surprising since you said that it was a no-op before.
(Assignee)

Comment 12

a year ago
Note that before, this was handled differently:
https://hg.mozilla.org/comm-central/diff/078dd301fa0c/mailnews/base/prefs/content/accountcreation/fetchhttp.js#l140
(Assignee)

Comment 13

a year ago
Aceman, can you take a look at this, https://wiki.mozilla.org/Thunderbird/Community_Members lists you for "account manager (UI)". This is old Bienvenu code, and the two lines in question here are by Joshua, see comment #12.
Flags: needinfo?(jonas) → needinfo?(acelists)
(Assignee)

Comment 14

a year ago
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #11)
> I commented out our
>   this._request.overrideMimeType("text/xml");
> and that does change the behaviour (it hangs completely now) which is
> surprising since you said that it was a no-op before.
Oh, ignore that, I must have mucked something up in the build. Taking the line out makes it work.
(Assignee)

Comment 15

a year ago
Created attachment 8763305 [details] [diff] [review]
This seems to work.

No idea who has expertise in this area. Apparently overrideMimeType() was a no-op, so no damage done removing it ;-) With this change, account creation works again.
Flags: needinfo?(acelists)
Attachment #8763305 - Flags: review?(mkmelin+mozilla)
Attachment #8763305 - Flags: review?(acelists)
Attachment #8763305 - Flags: feedback?(aleth)
(Assignee)

Comment 16

a year ago
Thomas, thanks for your help!

Comment 17

a year ago
Comment on attachment 8763305 [details] [diff] [review]
This seems to work.

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

I don't know this code, but I suspect this has revealed a bug that needs fixing (seeing bugs are referenced in the change history).

I'm OK with removing the noop to fix the test though, as that just restores the status quo.

::: mailnews/base/prefs/content/accountcreation/fetchhttp.js
@@ +134,5 @@
>          if (!mimetype)
>            mimetype = "";
>          mimetype = mimetype.split(";")[0];
>          if (mimetype == "text/xml" ||
> +            mimetype == "application/xml")

What happened to text/rdf?
Attachment #8763305 - Flags: feedback?(aleth)
(Assignee)

Comment 18

a year ago
(In reply to aleth [:aleth] from comment #17)
> What happened to text/rdf?
I thought rdf isn't xml, so not a good idea to pass it to JXON.build().

Comment 19

a year ago
Comment on attachment 8763305 [details] [diff] [review]
This seems to work.

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

I'd leave text/rdf

RDF has multiple serializations, one of which is XML. text/rdf without +subtype is basically wrong but as this is dealing with wrongly configured servers it's probably best to just leave it in.
Attachment #8763305 - Flags: review?(mkmelin+mozilla)
Attachment #8763305 - Flags: review?(acelists)
Attachment #8763305 - Flags: review+
(Assignee)

Comment 20

a year ago
Created attachment 8763314 [details] [diff] [review]
This seems to work (v2).
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #8763314 - Flags: review+
(Assignee)

Updated

a year ago
Attachment #8763305 - Attachment is obsolete: true
(Assignee)

Comment 21

a year ago
Aleth, please land at you leisure. No need to rush since today's nightly is already on the way.
Keywords: checkin-needed

Comment 22

a year ago
https://hg.mozilla.org/comm-central/rev/cb066e3a2889c0eb93fc1c84748615b065a9ee6c
Bug 1280682 - Remove call to overrideMimeType() due to bug 918733. r=mkmelin

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0

Comment 23

a year ago
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #16)
> Thomas, thanks for your help!

No problem, sorry the change crept up on you like that. I'm working on other XHR-related spec stuff, so I'll try to keep you guys in mind if other patches of mine might break this code for some reason.
You need to log in before you can comment on or make changes to this bug.