Closed Bug 1401026 Opened 3 years ago Closed 3 years ago

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

Categories

(MailNews Core :: Networking: IMAP, defect)

defect
Not set
normal

Tracking

(thunderbird_esr5257+ fixed, thunderbird56 fixed, thunderbird57 fixed)

RESOLVED FIXED
Thunderbird 57.0
Tracking Status
thunderbird_esr52 57+ fixed
thunderbird56 --- fixed
thunderbird57 --- fixed

People

(Reporter: Tonnes, Assigned: jorgk-bmo)

References

Details

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

Attachments

(3 files)

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.
I noticed that too. But it's not the e-mail address, but the account name, right? I'll look into it.
(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.
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
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.
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+
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: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 57.0
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?
Incredible, two tests also hard-code the incorrect message :-(
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d3dd512fb507
Follow-up: update test expectancy with server name. r=me DONTBUILD
Whiteboard: TB 56 beta 4 => TB 52.4.1 ESR
Whiteboard: TB 56 beta 4 => TB 52.4.1 ESR → TB 56 beta 4 => TB 52.5 ESR
Attachment #8910098 - Flags: approval-comm-esr52? → approval-comm-esr52+
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 :-(
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bdf2d5e6377f
Follow-up: Fix unsafe use of NS_ConvertUTF8toUTF16(). r=me DONTBUILD
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
Attachment #8910358 - Flags: review+
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+
See Also: → 1443739
You need to log in before you can comment on or make changes to this bug.