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

RESOLVED FIXED in Thunderbird 57.0

Status

defect
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: Tonnes, Assigned: jorgk)

Tracking

unspecified
Thunderbird 57.0

Thunderbird Tracking Flags

(thunderbird_esr5257+ fixed, thunderbird56 fixed, thunderbird57 fixed)

Details

(Whiteboard: TB 56 beta 4 => TB 52.5 ESR)

Attachments

(3 attachments)

Reporter

Description

2 years ago
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.
Assignee

Comment 1

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

Comment 2

2 years ago
(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.
Assignee

Updated

2 years ago
Summary: Some IMAP error messages contain email addresses where they should be server names → Some IMAP error messages contain account names where they should be server names
Assignee

Comment 3

2 years ago
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.
Assignee

Comment 4

2 years ago
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8910098 - Flags: review?(frgrahl)
Comment on attachment 8910098 [details] [diff] [review]
1401026-imap-alert.patch (v1)

Looks great.
Attachment #8910098 - Flags: review?(frgrahl) → review+

Comment 6

2 years ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3d6725fd0288
user real server name instead of account name (pretty name) for IMAP alerts. r=frg
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Assignee

Updated

2 years ago
Target Milestone: --- → Thunderbird 57.0
Assignee

Comment 7

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

Simple fix that will give correct alerts when IMAP connections fail.
Attachment #8910098 - Flags: approval-comm-esr52?
Assignee

Comment 8

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

Comment 9

2 years ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d3dd512fb507
Follow-up: update test expectancy with server name. r=me DONTBUILD
Assignee

Updated

2 years ago
Whiteboard: TB 56 beta 4 => TB 52.4.1 ESR
Assignee

Updated

2 years ago
Whiteboard: TB 56 beta 4 => TB 52.4.1 ESR → TB 56 beta 4 => TB 52.5 ESR
Assignee

Updated

2 years ago
Attachment #8910098 - Flags: approval-comm-esr52? → approval-comm-esr52+
Assignee

Comment 12

2 years ago
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

2 years ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bdf2d5e6377f
Follow-up: Fix unsafe use of NS_ConvertUTF8toUTF16(). r=me DONTBUILD
Assignee

Comment 14

2 years ago
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

Updated

2 years ago
Attachment #8910358 - Flags: review+

Comment 16

2 years ago
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.
Attachment #8930717 - Flags: review+
Assignee

Updated

2 years ago
See Also: → 1443739
You need to log in before you can comment on or make changes to this bug.