505589 - Movemail identity page shows useless @ sign behind email address box

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[Karsten Düsterloh]

Attachment: a domain is not an array

Bug:
Open Account Wizard, select "Movemail" (i.e. you need a unixoid OS!), select "Next". See "Email Address" box with a lone "@" behind it.

Okay, but ugly:
Open Account Wizard, select "Email Account", select "Next". See "Email Address" box with empty space behind it.

Okay:
Open Account Wizard, select "Gmail IMAP" (if present for you), select "Next". See "Email Address" box with "@gmail.com" behind it.

This is a regression from bug 400931.


The attached patch fixes the actual bug in ispUtils.js by assigning an actual string as the domain name and not an array. It also fixes the ugly empty space behind the "Email Address" box by hiding the domain field if unused, plus enlarging the textbox.

Asking for reviews from Magnus who did it ;-), from Bryan for TB theme changes and Neil for SM theme changes and overall superreview.

Comment 1

[Magnus Melin [:mkmelin]]

Comment on attachment 389813 [details] [diff] [review]
a domain is not an array

Looks good to me, r=mkmelin

On a side note, does Global inbox really work for movemail?

Comment 2

[Joshua Cranmer [:jcranmer]]

(In reply to comment #1)
> (From update of attachment 389813 [details] [diff] [review])
> Looks good to me, r=mkmelin
> 
> On a side note, does Global inbox really work for movemail?

No, it only works for POP, as the required attributes are limited to the POP interface. I have considered promoting them to nsIMsgIncomingServer, but when I last queried, there was debate about the possible lack of global inbox in the future.

Comment 3

[Magnus Melin [:mkmelin]]

Yeah I doubt anyone is really missing that option. The wiz should probably not offer it though.

Comment 4

[[e-mail]]

Comment on attachment 389813 [details] [diff] [review]
a domain is not an array

>+  postEmailText.collapsed = true;
>+  postEmailText.collapsed = false;
Nit: I'd prefer hidden to collapsed

>         var domainData = domainURI.split(':');
>         if (domainData.length > 1) {
>           // To faciltate distributing two different account types for one ISP,
>           // it's possible to add parameters to the domain URI 
>           // - e.g. domain:gmail.com?type=imap.
>           // This is necessary so RDF doesn't think they're the same.
> 
>           // Save the domain, but only the part up to the (possible) question mark.
>-          result.domain = /[^?]*/.exec(domainData[1]);
>+          result.domain = /[^?]*/.exec(domainData[1])[0];
>         }
Eww, what nasty string fu :-( I guess I can live with the split, but I'd prefer domainData[1].replace(/\?.*/, "") instead of abusing exec.

Comment 5

[Karsten Düsterloh]

Pushed as <http://hg.mozilla.org/comm-central/rev/e1654a638791> with requested changes of comment #4.