166068 - address field guess policy

Status: RESOLVED FIXED

(MailNews Core :: Composition, enhancement)

Description

[Eyal Rozenberg]

Currently, if you enter an address field like 'To'/'CC'/'Reply-To' as the last
address field and press enter, the new address field's type is set to the type
of the previous last address field. While this is reasonable for 'To', 'CC' and
'BCC' it is highly unlikely to assume that after entering a reply-to address
someone would enter another one.

Specifically, if I delete my first 'To' address the cursor goes up to my
'Reply-To' address, and when I press enter to type a different 'To' address I
have to change the field type from 'Reply-To' to 'To'.

So I believe after pressing enter in the last address field which is a Reply-To
field the field type should change to 'To'.

Please vote for this bug! I promise to cut/raise taxes and fight/make peace with
our evil enemies!

Comment 1

[Jean-Francois Ducarroz]

a totally approve. We should simply set the new field to "TO" or reuse the
previous valid one (TO, CC, BCC).

Over to varada

Comment 2

[Mike Cowperthwaite]

=>enhancement, confirming

Comment 3

[Eyal Rozenberg]

Attachment: proposed fix

I added a switch of the previous line's field type, which does not clone for
'reply-to' and 'other' addresses. Obviously To, CC and BCC are clonable; what
about 'newsgroup' and 'follow-up to' ? I'm not a usenetty person so I've never
used these.

Comment 4

[Mike Cowperthwaite]

Newgroup *could* be, altho that implies cross-posting which is generally not the 
best thing to do.  However, if there *is* a newsgroup header, that would be the 
only thing that sensibly *could* be cloned, since To/CC/BCC don't work (aren't 
supposed to work?) when a message is posted to a group.

Followup-To should only occur once.

Comment 5

[Eyal Rozenberg]

Ok, I'll wait for Neil's thoughts, then make the necessary change.

You know, I can't believe I was lazy enough these last 2 years to not go to the
trouble of finding out how to fix this. It's so trivial... (sigh)

Comment 6

[[e-mail]]

Comment on attachment 149101 [details] [diff] [review]
proposed fix

>-      select[0].selectedItem = select[0].childNodes[0].childNodes[awGetSelectItemIndex(lastRecipientType)];
Interestingly this caused a bug in itself - because all custom headers have the
same "other" recipient type only the last one cloned itself correctly.

>+      // should addr_newsgroups and addr_followup be cloned?
Newsgroups yes; followup-to no.

>+          select[0].selectedItem = select[0].childNodes[0].childNodes[awGetSelectItemIndex(lastRecipientType)];
Menulists have a selectedIndex property which should simplify this.

>+          break;        
>+        case "addr_reply"       : 
>+        case "addr_other"       : 
I'd prefer default: (without a trailing space!) here.

>+          select[0].selectedItem = select[0].childNodes[0].childNodes[awGetSelectItemIndex("addr_to")];
awGetSelectItemIndex("addr_to") should always be 0, I hope?

Comment 7

[Eyal Rozenberg]

Attachment: proposed fix v2

reshuffled switch cases, a little less kludgy index retrieval, and a comment
suggesting awGetSelectItemIndex should be replaced with something better.

Comment 8

[Eyal Rozenberg]

(after consulting with Neil Rashbrook and gazing at the code for a bit)

The current policy of function loadStartFolder is to get the default account,
and if it has 'check for new messages at startup' set, expand
it and check for new messages on it (perform 'Biff').

The new policy I intend to implement is, first, to go over _all_ accounts and
check for
messages if they have 'check for new messages at startup' set.
Second, decide which account gets focus, and whether
to focus on its inbox (in which case it also gets expanded
if it collapsed) or on the 'account central'. The logic
of these decisions is best demonstrated by example (which Neil has graciously
devised):

Let P the first, default, account; let I the second.
    
The letter in the middle of each table entry indicates which
account gets focus. Each entry also lists one of the
following:

'act cen' - focus on the account's 'account central'
'inbox' - focus on the account's inbox


+---------------+---------------+---------------+
|               |Check for msgs |No check       |
|             P +-------+-------+-------+-------+
|       I       |Open   |Closed |Open   |Closed |
+-------+-------+-------+-------+-------+-------+
|Check  |Open   |   P   |   I   |   I   |   I   |
|       |       | inbox | inbox | inbox | inbox |
|for    +-------+-------+-------+-------+-------+
|msgs   |Closed |   P   |   P   |   I   |   I   |
|       |       | inbox |act cen|act cen|act cen|
+-------+-------+-------+-------+-------+-------+
|No     |Open   |   P   |   P   |   P   |   P   |
|       |       | inbox |act cen|act cen|act cen|
|Check  +-------+-------+-------+-------+-------+
|       |Closed |   P   |   P   |   P   |   P   |
|       |       | inbox |act cen|act cen|act cen|
+-------+-------+-------+-------+-------+-------+ 

Note that default account has priority, then priority is by order in the tree. I
don't know whether the default account can be anything other than the first
account, that remains to be seen.

Comment 9

[Eyal Rozenberg]

whoops, commented in the wrong bug. sorry.

Comment 10

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

Checking in mailnews/compose/resources/content/addressingWidgetOverlay.js;
/cvsroot/mozilla/mailnews/compose/resources/content/addressingWidgetOverlay.js,v
 <--  addressingWidgetOverlay.js
new revision: 1.88; previous revision: 1.87
done

Comment 11

[Eyal Rozenberg]

biesi says: patch checked in.