The default bug view has changed. See this FAQ.

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
9 months ago
9 months ago

People

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

Tracking

({regression})

Trunk
Thunderbird 50.0
regression

Thunderbird Tracking Flags

(thunderbird49 unaffected, thunderbird50 affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months 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

9 months 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

9 months 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

9 months 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

9 months 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

9 months 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

9 months 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

9 months 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

9 months ago
status-thunderbird49: --- → unaffected
status-thunderbird50: --- → affected
Keywords: regressionwindow-wanted

Updated

9 months ago
Component: General → Account Manager
(Assignee)

Comment 8

9 months 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

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

Comment 10

9 months 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

9 months ago
Flags: needinfo?(wisniewskit)
(Assignee)

Comment 11

9 months 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

9 months 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

9 months 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

9 months 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

9 months 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

9 months ago
Thomas, thanks for your help!

Comment 17

9 months 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

9 months 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

9 months 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

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

Updated

9 months ago
Attachment #8763305 - Attachment is obsolete: true
(Assignee)

Comment 21

9 months 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

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

Updated

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

Comment 23

9 months 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.