285626 - Advanced POP settings dialog is too small

Status: VERIFIED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Mike Cowperthwaite]

Now that bug 202468's patch has landed, this issue has arisen.
TB 1.0+0309, also Moz 1.8b2-0309.


The advanced settings for a POP server initially show only a few radio buttons, 
and the window where these now reside is sized to fit them (actually, I think it 
looks a little cramped).

But if you select either of the non-default settings -- to defer the account to 
either Local Folders or another account's Inbox -- the controls change 
dynamically (which is a design issue I disagree with, but that's another bug).  
With the new controls in either of those other settings, the window is too small 
and truncates parts of the buttons.

Comment 1

[Doug Wright]

I can see this too - the window is the right size when its initial state is with
the drop-down, but not otherwise.

Comment 2

[Stefan Borggraefe]

The problem is that the dialog doesn't incorporate the hidden elements when
calculating its initial size. Thus these elements do not fit inside of the
dialog when they are shown at a later point.

We cannot set a good fixed default size for this dialog, because the length of
the account name is variable and so the menulist also has a variable width.

I tried to come up with a modified UI that doesn't have this problem. I am going
to upload a screenshot and a patch in an minute.

Comment 3

[Stefan Borggraefe]

Attachment: Screenshot

The "Global Inbox" radio button can go because the menulist already includes
the Local Folders account and the description above explains that this account
contains the "Global Inbox".

Comment 4

[Stefan Borggraefe]

Attachment: Patch

David, I think you introduced the "Global Inbox" UI. Do you like this change?

Comment 5

[David :Bienvenu]

No, I really wanted global inbox to be its own choice, so it would be really
easy for people to pick that. It will be by far the most common choice. Having
the account drop down default to local folders is nice, but I still prefer
having a distinct global inbox choice. I don't know how hard this bug is to fix
otherwise, though.

Comment 6

[Stefan Borggraefe]

David, I see your point. Another possible way to fix this bug would be to just
disable the elements instead of hiding them.

Comment 7

[Mike Cowperthwaite]

(In reply to comment #6)
> just disable the elements instead of hiding them.

That was something I thought of when these options were first introduced; and I 
still dislike dynamically reflowing of dialog UI.  However, I don't know of 
another radio option that disables subordinate controls -- except for disabling 
a dropdown like the account selector in these controls.

How about something like this instead (compare the "Handling" group of Junk Mail 
Controls):

   When downloading pop3 mail for this server, you can store it in an
   alternate Inbox.  Selecting this option hides this account in the
   folder pane.
   [] Store this account's mail in:
      () Global Inbox
      () Inbox of [Local Folders |v|]

      [] Include this server when getting new mail


btw, wouldn't it be better if the Include checkbox were checked by default?

Comment 8

[Alex Ihrig [:Thunderbird_Mail_DE]]

The dialogs size is too small, when localizing this dialog, too!

If people can't use this dialog, it is more than a minor problem > in some cases
they would not be able to configure there accounts and (if they have choosen a
false option in the account wizard before) they would not be able to access
there mail.

This is a major problem!

Comment 9

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: Screenshot localized Thunderbird DE

Comment 10

[Alex Ihrig [:Thunderbird_Mail_DE]]

One more thing to mention:

The height and the width can be (is) too small, if you choose different options
in the dialog.

Comment 11

[Scott MacGregor]

Attachment: new screen shot with a fix

Here's a fix for 1.1 that fixes the dialog size and disables elements instead
of hiding them.

Comment 12

[Scott MacGregor]

Attachment: the fix

fix that makes the previously posted screen shot possible.

Comment 13

[Stefan Borggraefe]

Comment on attachment 186271 [details] [diff] [review]
the fix

>+<!ENTITY pop3Desc.label "When downloading pop3 mail for this server, use the following folder for new mail:" >

I think "POP" instead of "pop3" would be better. This would be consistent with
the main Server Settings pane.

>-    {
>-      picker.hidden = false;
>-      picker.removeAttribute("disabled");
>-    }
>+      picker.removeAttribute('disabled');
>     else
>-    {
>-      picker.hidden = true;
>-    }
>+      picker.setAttribute('disabled', true);
>+
>     var deferCheckbox = document.getElementById('deferGetNewMail');
>-    deferCheckbox.hidden = !showDeferGetNewMail;
>+    if (showDeferGetNewMail)
>+      deferCheckbox.removeAttribute('disabled');
>+    else
>+      deferCheckbox.setAttribute('disabled', true);  
> }

You can use the disabled property instead of the attribute here. Also "enable"
instead of "show" would make more sense now. Looks like the "event" parameter
of this function is unused.

>Index: mailnews/base/prefs/resources/content/am-server-advanced.xul
>===================================================================
>          <row>
>             <radio value="1" id = "accountDirectory" label="&accountDirectory.label;"
>                  accesskey="&accountDirectory.accesskey;"
>                  oncommand="updateInboxAccount(false, false)" />
>          </row>
>+           <row align = "center">
>+            <radio value="0" id = "globalInbox" label="&globalInbox.label;"
>+                   oncommand="updateInboxAccount(false, true)" 
>+                   accesskey="&globalInbox.accesskey;"/>
>+           </row>
>          <row>
>             <radio value="2" id = "deferToServer" label="&deferToServer.label;"
>                  accesskey="&deferToServer.accesskey;"
>-                 oncommand="updateInboxAccount(true, true)" 
>-                 />
>+                   oncommand="updateInboxAccount(true, true)"/>

Please fix the indentation.

r=me with these small changes. :-)

Comment 14

[(not reading, please use [e-mail] instead)]

Comment on attachment 186271 [details] [diff] [review]
the fix

a=sspitzer

the new screen shot looks good.

Comment 15

[Scott MacGregor]

fixed. I also addressed Stefan's comments.

Comment 16

[Mike Cowperthwaite]

Verified fixed with TB 1.0+0618, Win2K (large fonts).
Thanks, Scott.