1401026 - Some IMAP error messages contain account names where they should be server names

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Ton [:Tonnes]]

Thunderbird version=56.0a1
BuildID=20170801030207

For IMAP, at least 2 error messages exist that contain email addresses where they should be server names instead, knowing

- Server %S has disconnected. The server may have gone down or there may be a network problem. 
- Failed to connect to server %S. 

%S is obviously replaced by the user’s email address instead of the account’s server name.

Both strings are in ImapMsgs.properties, and I’m not sure if the issue is limited to these 2. I do know it looks odd and can be confusing to users as well as support helpers who may assume users have put their email address in the server name field.

Please check and replace any variables to include the proper server names.

Comment 1

[Jorg K (CEST = GMT+2)]

I noticed that too. But it's not the e-mail address, but the account name, right? I'll look into it.

Comment 2

[Ton [:Tonnes]]

(In reply to Jorg K (GMT+2) from comment #1)
> I noticed that too. But it's not the e-mail address, but the account name,
> right? I'll look into it.

Correct, they happen to be similar in my setup. :) Thanks.

Comment 3

[Jorg K (CEST = GMT+2)]

For the record:
imapUnknownHostError=Failed to connect to server %S. used in mailnews/imap/src/nsImapProtocol.cpp
imapServerDisconnected=Server %S has disconnected. ... used there and in nsImapServerResponseParser.cpp

I'll check it.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1401026-imap-alert.patch (v1)

I wrote this on the plane so I had a good opportunity to test the connection failures. Please try this in SM, it's a tiny change.

Comment 5

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8910098 [details] [diff] [review]
1401026-imap-alert.patch (v1)

Looks great.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3d6725fd0288
user real server name instead of account name (pretty name) for IMAP alerts. r=frg

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8910098 [details] [diff] [review]
1401026-imap-alert.patch (v1)

Simple fix that will give correct alerts when IMAP connections fail.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1401026-follow-up.patch

Incredible, two tests also hard-code the incorrect message :-(

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d3dd512fb507
Follow-up: update test expectancy with server name. r=me DONTBUILD

Comment 10

[Jorg K (CEST = GMT+2)]

Beta (TB 56):
https://hg.mozilla.org/releases/comm-beta/rev/1e947b81c3f74b8a4eff17186bf7a74b30f1499c
https://hg.mozilla.org/releases/comm-beta/rev/eca90d4a27d2e1e30c7ddcfddddda5be2227b967

Comment 11

[Jorg K (CEST = GMT+2)]

TB 52.5 ESR (should be tracking 57+):
https://hg.mozilla.org/releases/comm-esr52/rev/0468782a24a2
https://hg.mozilla.org/releases/comm-esr52/rev/315eb1f87131

Comment 12

[Jorg K (CEST = GMT+2)]

Sadly this gave test failures since the error message was never correct. This is due to
  const char16_t *params[] = { NS_ConvertUTF8toUTF16(hostName).get() }
where the NS_ConvertUTF8toUTF16(hostName) has no reference and goes out of scope immediately :-(

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bdf2d5e6377f
Follow-up: Fix unsafe use of NS_ConvertUTF8toUTF16(). r=me DONTBUILD

Comment 14

[Jorg K (CEST = GMT+2)]

I've actually discussed that change with Eric Rahm on IRC:
erahm: jorgk: FWIW you want something like: const NS_ConvertUTF8toUTF16 foo(aCSubstring); ... params = { foo.get(); }
so that's exactly what I've done.

C-B: https://hg.mozilla.org/releases/comm-beta/rev/8a0947246021f21690004c214b344cd7891b23de
C-esr52: https://hg.mozilla.org/releases/comm-esr52/rev/49755a7bb25b7a2c48b664e29cc63ecf40488d53

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: 1401026-follow-up.patch - another follow-up fixing unsafe NS_ConvertUTF8toUTF16() usage

Comment 16

[:aceman]

Comment on attachment 8930717 [details] [diff] [review]
1401026-follow-up.patch - another follow-up fixing unsafe NS_ConvertUTF8toUTF16() usage

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

We discussed this on IRC so thanks for finding this out and the fix.