77202 - Default POP account leaves an empty thread pane & msg pane

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[Sheela Ravindran]

Buildid:  2001041908 linux

When we create a new profile with pop as default account with pref'check mail at 
startup' checked by default selects the inbox and displays a blank thread pane. 
From the above build I had both thread pane and message pane blank.  However I 
know that we display start page in the message pane which probably was broken in 
the above build. Biff arrow at the account level is the only action or 
indication in the 3 pane window that happens after logging in. But for a first 
time user and also for not an experienced pop user when inbox is selected in the 
folder pane and seeing a blank thread pane is probably not very user friendly.

Steps to see the above problem:
Create a new profile
From tasks menu item open mail
Create the first or default account as pop 
When the password dialog appears - log in with the correct password

Actual Results:  Green arrow appears at the account level.  Inbox is selected in 
the folder pane.  Thread pane is blank.  No update in the status bar. If there 
is a problem displaying start page in the message pane then we have the right 
side two blank panes.

Expected Results:  Other than just a green arrow indication, provide some useful 
information as to why he/she is not seeing anything on the thread pane even 
though his inbox is selected. The argument would be that pop users know that 
they have to click get message to actually get messages or see messages.  But 
still the first time creation of an account and for the new pop user it is not 
very informative.  We build the Account central page with so much information 
but we are not making much use of it for atleast pop at this point.

Comment 1

[Sheela Ravindran]

change qa contact to myself and cc-ing all the related people.  

Comment 2

[Sheela Ravindran]

Spoke to Ninoschka and reassigning to her since this more UI and an account
manager bug.  

Comment 3

[Ninoschka Baca]

Problem Summary: Create a new POP account, login, the green arrow appears on the
account level and no messages appear in the thread pane. 

Suggestion: The new user may be confused at this point. When this happens maybe
we could have some text which states "You are logged into your Mail account.
Select the Get Msg button for messages to appear". This statement would only be
needed the first time the user creates a new POP account.

Comment 4

[Ninoschka Baca]

Marking nsbeta1 to make the POP user's first experience easier.

Comment 5

[jglick]

Yeah POP accounts are weird. We check for new messages but don't pull them. I 
can see the potential benefits of this in an everyday usage scenario, but you'd 
think if the user just created a brand new account, they'd want to pull 
their messages.

We could either:
1. On acct creation only, pull the messages (probably upset the strick POP 
folks)
2. At the completion of creating a new POP account, open a dialog that asks 
"Would you like to download your messages now?".
3. As nbaca suggested, display in the thread pane text such as, "You are logged 
into your Mail account. Select the Get Msg button for messages to appear".

How did 4.x handle this?

Comment 6

[Ninoschka Baca]

Using 4.7 I created a new profile, configured for POP, when Messenger opens I
see the folders and a blank thread pane. Nothing else happens so the user needs
to select the Get Message button to see messages. 

Comment 7

[scottputterman]

marking nsbeta1+ and moving to 0.9.2. I don't know if it will make it but I
don't want to lose it yet.

Comment 8

[scottputterman]

moving to 0.9.3.  We need to talk about this one a bit more.  It may go back to
0.9.2

Comment 9

[scottputterman]

moving to 0.9.4

Comment 10

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

slide to 0.9.5

Comment 11

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

moving out.

Comment 12

[scottputterman]

How about #2 or #3 from Comment 5?  Reassigning to shliang.

Comment 13

[shliang]

Attachment: patch

adds option at end of account wizard for new default pop account to download
messages - screenshot coming up.

Comment 14

[shliang]

Attachment: screenshot

last page in account creation wizard

Comment 15

[kmurray1115]

We should proabably have it checked (on) by default. Also, does this appear for
all account types or just POP?

Comment 16

[jglick]

Can text be changed to "Download new messages now"? I agree, on by default. More 
knowledgeable users who don't want their messages downloaded will know to 
uncheck option.

As for all accts (IMAP, NNTP) vs POP, is there a benefit for IMAP, News to have 
this pref?

Comment 17

[shliang]

this is just for pop accounts, only if it is the first one created after 
creating a new profile (that's what i thought it was supposed to be, from 
reading the above comments).

imap already downloads messages automatically i think. the way i did it, so 
far, it will only download new messages the very first time the user logs in, 
right after creating the account. i wasn't sure if it was supposed to do that 
just once, or every time. i guess if the text says "Download new messages now" 
then it should only happen that one time. 

jen, can you let me know what other changes you want me to make in addition to 
the text change and having the box checked by default? thanks :)

Comment 18

[jglick]

Shuehan, just verified with Putterman:

1. POP accounts only
2. All new POP accounts, not just for the first new profile POP acct (since this 
is a problem with any newly created POP acct)
3. Only download new messages the very first time the user logs in, right after 
creating the account.

That's it. :-) 
Thanks you.

Comment 19

[shliang]

Attachment: patch

adds option to download new messages for all new pop accounts. 

text changed to "Download new messages now" and is checked by default.

Comment 20

[Sean Su]

Comment on attachment 74172 [details] [diff] [review]
patch

The patch looks okay to me, but I'm not that familiar with the account wizard
code.  You should definitely run this by bhuvan though.

The one thing that I did notice is that you might want to not call
defaultServer.PerformBiff() in msgMail3PaneWindow.js when:
  (defaultServer.type != "imap") && !gNewDefaultPopGetMsg

I'm wondering if calling biff would interfere with the downloading of new
messages?

Comment 21

[Sean Su]

Comment on attachment 74172 [details] [diff] [review]
patch

I just ran thru the patch.  Works really good, except for one minor thing. 
Move the following code from loginPageInit() in aw-login.js:

 +	// add option to download new messages if this account is a new pop
account
 +	if (type == "pop3") {
 +	    document.getElementById("downloadMsgs").hidden = false;
 +	}
 +

to setServerType() in aw-server.js.  And place an else{} so that it can set it
to true as well as false.

The reason for this is that if the user goes thru the account wizard and
selects IMAP, the checkbox will be hidden.  This is correct.  But if the user
then decides to go back thru the dialogs and change from IMAP to POP3, it will
still be hidden.  The same problem goes for changing from POP3 to IMAP.  Moving
the checking code into setServerType() will fix this problem.

r=ssu once this is moved.

Comment 22

[shliang]

Attachment: updated patch

Comment 23

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

1)

+
+function CheckGetMsgForPop(newAccount)
+{
+  if (document.getElementById("downloadMsgs").checked) {
+    var mailUrl = "chrome://messenger/content/messenger.xul";


+function CheckNewPopGetMsg()
+{
+  if (location.href == "chrome://messenger/content/messenger.xul") {
+    if (gNewPopAcctGetMsg && gNewPopAcct) {

messenger.xul is not the only xul for the 3 pane.  

Right now, we also have the alternative 3 pane xul, mail3PaneWindowVertLayout.xul
so we'll need to take care of that scenario, too.

2)

the comment doesn't match the code

             // Perform biff on the server to check for new mail, except for imap
-            if (defaultServer.type != "imap")
+            if ((defaultServer.type != "imap") && !gNewDefaultPopGetMsg)
               defaultServer.PerformBiff();         
         } 
 
3)

please add a comment to the calls to CheckNewPopGetMsg(), something like:
if a pop account was created, and the user checked "download messages"
we'll need to download the messages for that account.

 window.openDialog("chrome://messenger/content/AccountWizard.xul",
                         "AccountWizard", "chrome,modal,titlebar,resizable");
 
+    CheckNewPopGetMsg();
 }
 
 // selectPage: the xul file name for the viewing page, 
@@ -213,4 +214,38 @@
     window.openDialog("chrome://messenger/content/AccountManager.xul",
                       "AccountManager", "chrome,modal,titlebar,resizable",
                       { server: server, selectPage: selectPage });
+
+    CheckNewPopGetMsg();

4)

+ // Going thru open windows because we don't know if account wizard was opened
+ // by the mail window or account manager.

this is so we don't execute this code if from addressbook, compose, right?

In that case, if the user checks the check box to "download" messages for
the new pop account created from addressbook or mail compose, we won't download.

In that scenario, should we open a 3 pane, select the inbox, and then download
messages? This issue could be spun off to another bug.

5)  

what if the user creates 2 pop accounts at the same time from the account
manager? It looks like we'll only download new mail for the last pop account
created.  Is that acceptable?  This issue could be spun off to another bug.

6)

shouldn't we be re-setting gNewPopAcct and gNewPopAcctGetMsg to null / false
after we download new messages?

Comment 24

[shliang]

Attachment: patch

4) 

how do you create a new account from addressbook? i can't seem to find it.

from compose, the checkbox won't be displayed in the account wizard. i decided
to do this for now because the bug was originally created so that the user does
not see a blank thread pane after creating a new pop account, so if messenger
isn't open in the first place, this wouldn't be a problem. or should we just
open one up and download the messages?

5) 

this will only download messages for the last pop account created. is that ok?

Comment 25

[[e-mail]]

Comment on attachment 77581 [details] [diff] [review]
patch

Nits:
Glad to see you using .hidden, but forgot to remove the code from
loginPageInit().
CheckNewPopGetMsg is overkill, you just need to call
GetMessagesForInboxOnServer(gNewPopAcct.incomingServer);
You don't need gNewPopAcctGetMsg because gNewPopAcct is null whenever
gNewPopAcctGetMsg is false so you only need to test that instead.
I don't see the point gNewDefaultPopGetMsg - you never set it anywhere. You
also clear gNewPopAcct which you could otherwise use instead.

Comment 26

[Ninoschka Baca]

Due to bug# 134158, in a new profile, create a POP account, login and it
automatically retrieves messages that are seen in the thread pane. Add a second
POP account, login, biff appears on the server but it does Not automatically
download messages. 

Because of this change, the fix for this bug would effect additional POP
accounts and not the first account? Do we still want to fix this bug?

Comment 27

[shliang]

Attachment: updated patch

option to download msgs upon acct wizard close will appear for all new pop
accounts

Comment 28

[Cavin Song]

Comment on attachment 113041 [details] [diff] [review]
updated patch

r=cavin.

Comment 29

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

see nbaca's last comment.

http://bugzilla.mozilla.org/show_bug.cgi?id=77202#c26

if we add this code, but disable the checkbox, it sounds like we'd still
download the messages.

Comment 30

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

Comment on attachment 113041 [details] [diff] [review]
updated patch

what about nbaca's comment?

it sounds like I could have the "download new messages now" disabled, yet due
to bug #134158, I'd still download.

also, what if I was offline?

Comment 31

[shliang]

Attachment: patch

it sounds like I could have the "download new messages now" disabled, yet due
to bug #134158, I'd still download.
- ok, fixed this

also, what if I was offline?
- you just get the usual can't connect messages

Comment 32

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

> also, what if I was offline?
> - you just get the usual can't connect messages

use the io service to detect if offline or not.  if offline, disable that UI.

Comment 33

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

> it sounds like I could have the "download new messages now" disabled, yet due
> to bug #134158, I'd still download.
> - ok, fixed this

was your fix to do this:

gCurrentAccount.incomingServer.downloadOnBiff = false;

if so, that's not what we want.  we still want pop servers, by default, to
download on biff.

we should think about this, I'm not sure this patch is the right approach.

I'll investigate and comment later tonight or tomorrow.

I'm not sure about this patch

Comment 34

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

Comment on attachment 118255 [details] [diff] [review]
patch

this needs work.

Comment 35

[shliang]

Attachment: patch

Comment 36

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

Comment on attachment 120395 [details] [diff] [review]
patch

shuehan is working on a few minor issues, as will be attaching a new patch.

Comment 37

[shliang]

Attachment: patch

Comment 38

[shliang]

Attachment: patch

Comment 39

[shliang]

Attachment: patch

Comment 40

[shliang]

Comment on attachment 120674 [details] [diff] [review]
patch

sr=sspitzer

Comment 41

[shliang]

test cases:

1. new profile
 a. open from compose
 b. open from mail
 c. open from addressbook
2. file -> new account (from mail and message windows)
3. account manager -> add account

only pop, opened from mail directly, and online, 
should have download option. new pop account from new profile,
if unchecked, should not download.

Comment 42

[shliang]

resolving

Comment 43

[[e-mail]]

Comment on attachment 120674 [details] [diff] [review]
patch

>+    var inboxFolder = rootMsgFolder.getFoldersWithFlag(0x1000, 1, outNumFolders);      
>+    var selectedFolderResource = inboxFolder.QueryInterface(Components.interfaces.nsIRDFResource);
>+    var selectedFolder = selectedFolderResource.QueryInterface(Components.interfaces.nsIFolder);
>+    SelectFolder(selectedFolder.URI);
Actually inboxFolder is already an nsIMsgFolder so you can simply
SelectFolder(inboxFolder.URI);
here :-P

Comment 44

[Christian :Biesinger (don't email me, ping me on IRC)]

+        <checkbox id="downloadMsgs" label="&downloadOnLogin.label;"
checked="true"/>

hm, where are the .dtd changes for this entity? lxr doesn't find them either:
http://lxr.mozilla.org/seamonkey/search?string=downloadOnLogin.label

Comment 45

[jag (Peter Annema)]

Checked in the dtd entity as per one of the older patches to fix a blocker.

Comment 46

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

Attachment: supplimental fix

Comment 47

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

fix text (per mscott, we'll download all your messages on the server, not just
the new ones) and fix js (per neil).  this has r=shuehan

Comment 48

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

I'm seeing the checkbox for news accounts, but it should be a simple fix.

testing it now...

Comment 49

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

Attachment: another supplimental fix, for news accounts.

Comment 50

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

Attachment: better fix

this will work for news, imap, pop, and isp created accounts.

Comment 51

[Ninoschka Baca]

Trunk build 2003-04-22: WinXP, Mac 10.1.5 (haven't checked linux yet)

- New profile, open Mail, Download checkbox present and selected for first POP
account and the password dialog appears and the messages are downloaded.
- New profile, open Mail, Download checkbox present and selected, unchecked for
first POP account and password dialog does not appear as expected.

* New profile, from the Browser perform a Send Page and configure for a POP
account and the download checkbox is Not present (same problem occurs after
creating a new message from the browser or when the Address Book is open)
Reopening due to this last issue.

Comment 52

[shliang]

the checkbox isn't supposed to be there unless the account wizard was opened 
from the mail window

Comment 53

[shliang]

closing again, see above comment

Comment 54

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

to elaborate, here are the various ways you'd get the mail account wizard:

1)  no accounts, launch three pane (*)
2)  no accounts, launch addressbook
3)  no accounts, launch compose window
4)  stand alone msg window, File | New Account
5)  3 pane, File | New Account (*)
6)  from account manager, Add Account

in the case of 1 and 5, yes, we want to show the checkbox UI, as if you check
it, we have a three pane right there to be used.

in the case of 2, 3, 4, we don't have a three pane to use, so we'd have to open
one to properly show the user the downloading messages, parent the password
prompt, etc.

for case 6, we might have have opened the account manager from the 3 pane, but
the user could create multiple pop accounts, which gets messy.

So, shuehan's fix is designed to improve the initial experience:

if you create a pop account when mail first launches, allow the user to download
 messages.

You get the same experience if you do File | New Account from the 3 pane, but
you wouldn't (as I pointed out) if you created an account from the account
manager, which might be more common.

this is works as designed, and we decided to keep it simple.

Comment 55

[Ninoschka Baca]

Ok, thanks for the explanation.

Veriifed Fixed.