791542 - [TB17 test] IM account troubleshooting information is erroneous

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Vincent (caméléon)]

1/ Set-up a chat account (IRC or Google for instance)
2/ Display the troubleshoting informations
3/ Server name is not displayed : (im) realHostName FIXME:0

This was working good in TB15.

Comment 1

[Florian Quèze [:florian]]

(:aceman from bug 783457 comment #3)
> While testing it out I noticed my IM account (of type "JS test") shows
> garbage as:
> (im) realHostName FIXME:0
> 
> Probably realHostName is unimplemented. Also the port is shown as 0. The
> about:support generator hides the port only when it is set to -1. That is
> used in Local Folders and Feeds account types.
> Florian, please look at it if this isn't a problem in more IM protocols (or
> just in JS test).

Comment 2

[:aceman]

The bug 783457 was done for TB17, so 16 should be good too.

Comment 3

[Florian Quèze [:florian]]

Attachment: Patch

Comment 4

[:aceman]

Comment on attachment 661783 [details] [diff] [review]
Patch

Florian, for other server types, realHostName differs from hostName after it is changed from the initial value (at creation time).

For IM accounts, is the hostName also changed anytime the user changes it in Account manager?

When I now created new IM accounts I get prpl-irc and prpl-jabber as values for mail.server.server*.hostname (and the hostName getter seems to just fetch the pref). How does this work? Where is the hostname of the server?

Comment 5

[Florian Quèze [:florian]]

(In reply to :aceman from comment #4)
> Comment on attachment 661783 [details] [diff] [review]
> Patch
> 
> Florian, for other server types, realHostName differs from hostName after it
> is changed from the initial value (at creation time).
> 
> For IM accounts, is the hostName also changed anytime the user changes it in
> Account manager?

It's not possible to change the username of an IM account after it's been created. That's why we don't have 2 different values (userName and realUserName) like mailnews has to do for hostnames.
Of course some users complain about it :).

> When I now created new IM accounts I get prpl-irc and prpl-jabber as values
> for mail.server.server*.hostname (and the hostName getter seems to just
> fetch the pref). How does this work? Where is the hostname of the server?

In some specific cases (IRC, generic XMPP) the concept of hostname makes sense for IM accounts, but in general it doesn't (the hostname doesn't make sense for a twitter or an AIM account for example), so there's no generic way to access it.

I pondered showing the hostName (ie the id of the protocol plugin), the username, or both, but if I understood correctly, we want users to be able to copy and paste the whole troubleshooting page without leaking personal data, so showing the username isn't great.

Comment 6

[:aceman]

Can the specific IM account types override these default functions in imIncomingServer.js ?

For IRC the server host name AND port would be valuable to show on the troubleshooting page.

Comment 7

[Florian Quèze [:florian]]

(In reply to :aceman from comment #6)
> Can the specific IM account types override these default functions in
> imIncomingServer.js ?

The code implementing the protocol plugins (in chat/protocols/) can't (it doesn't know anything about what's in mail/components/im/ anyway).

We could add some protocol specific code in imIncomingServer.js to return better values for IRC and XMPP if we really wanted, but that's ugly, and largely out of the scope of a regression fix.

> For IRC the server host name AND port would be valuable to show on the
> troubleshooting page.

For both IRC and XMPP, having information in that page about the SSL settings would also be useful.

Comment 8

[:aceman]

Comment on attachment 661783 [details] [diff] [review]
Patch

OK, so as a quick fix to get into TB17 I am OK with this.
But to make the troubleshooting page useful for IM accounts, you could make a new bug for showing the host, port and connection security for the IM types where it is useful.

Comment 9

[Vincent (caméléon)]

it may make sens to use a different tab for mail accounts, another for IM accounts, and maybe in the future another for calendar. But I agree that this is another bug...

Comment 10

[Florian Quèze [:florian]]

Comment on attachment 661783 [details] [diff] [review]
Patch

This is in a sub module owned by chat/ peers, but I think the only person who has actually looked at this code before is David, so requesting review from him.

Comment 11

[David :Bienvenu]

Comment on attachment 661783 [details] [diff] [review]
Patch

looks reasonable.

Comment 12

[Florian Quèze [:florian]]

Comment on attachment 661783 [details] [diff] [review]
Patch

[Approval Request Comment]
Regression caused by (bug #): bug 783457

This is a regression in Thunderbird 17, and the patch is trivial, so I think we want this fix in comm-aurora.

Comment 13

[Florian Quèze [:florian]]

https://hg.mozilla.org/comm-central/rev/2f9e9cf80371

Comment 14

[Florian Quèze [:florian]]

https://hg.mozilla.org/releases/comm-aurora/rev/86c9974a8d0d