User Agent: Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0 Build ID: 20130409194949 Steps to reproduce: Typed mailing list name in "to:" field. Actual results: Send fails. Reports email address must be followed by @... Expected results: mail should have been sent to addresses in list. If a messages is created from the address book, individual names populate the "to:" boxes.

Exact message is: "XXXX Internal Distribution List <"Internal Email List for Document Distribution"> is not a valid e-mail address because it is not of the form user@host. You must correct it before sending the e-mail." Problems remains in version 24. Previously, this feature worked.

Confirmed that this failure happens for me also - version is 23a2 from Aurora channel for linux 64 bit as of yesterday's build. Was working fine in 22a2.

Is the problem that the address list does not expand to individual addresses when entered, or does it expand but you nevertheless get the error trying to send it?

It is hard to know since in the compose window you only see the list name and the name in chevrons, but not the list of addresses at that point, but you don't get the chance to proceed further before the popup complains and you can't do anything beyond that. Only once it has been sent can you see the list of addresses for the recipients in the copy of the mail in the Sent folder.

Moving this to the Address Book component as it's definitely related to the autocomplete function somehow.

(In reply to rsx11m from comment #3) > Is the problem that the address list does not expand to individual addresses > when entered, or does it expand but you nevertheless get the error trying to > send it? It appears Thunderbird is interpreting the entry as an individual email address rather than a mailing list. From the address book, if you click on a list, it opens a compose window with all of the individual addresses shown, but if you attempt to send from Thunderbird's main screen, it auto completes the list name, but appears not to properly link to the list entries.

Not seeing the addresses during compose in the compose window is normal and is the case with 22a2 as well - in other words for a list name, say, mylist, entering that in the To: field gives mylist <mylist>to the right of the To:, and you do not see the expanded list of addresses in the compose window. Once the mail is sent then checking that mail in the Sent folder will then show the expanded list of email addresses corresponding to the list name mylist. That is the behaviour I have seen for quite some time so when I saw that in 23a2 I was not surprised. However you can't get to the point of sending the email in 23a2 so you can't see the expanded list - in 22a2 you could not see the expanded list either but the email would send just fine and then you can see the full list of recipients only after it has been processed and sent on to the MTA.

A recent checkin matching the 23.0 time frame was bug 68784 which moved checking the recipient completeness before other checks, specifically the presence of a subject line. aceman, can you have a look if maybe that check went too much upfront and prevents the address list from properly expanding prior to sending now? If that's the issue, it should work with a trunk build prior to April 15 and fail with an April 16 or later build. If I read GenericSendMessage() correctly, it still executes Recipients2CompFields() before CheckValidEmailAddress(), thus msgCompFields should contain the already auto-completed addresses (unless I'm mistaken in assuming that address lists are handled there).

Is it possible that the problem lies in the lines: - // crude check that the to, cc, and bcc fields contain at least one '@'. - // We could parse each address, but that might be overkill. - if (to.length > 0 && (!to.contains("@", 1) && to.toLowerCase() != "postmaster" || to.endsWith("@"))) - invalidStr = to; - else if (cc.length > 0 && (!cc.contains("@", 1) && cc.toLowerCase() != "postmaster" || cc.endsWith("@"))) - invalidStr = cc; - else if (bcc.length > 0 && (!bcc.contains("@", 1) && bcc.toLowerCase() != "postmaster" || bcc.endsWith("@"))) - invalidStr = bcc; which comes from https://bug68784.bugzilla.mozilla.org/attachment.cgi?id=733026 i.e. the check seems to "require" that addresses contain "@" which would not apply to a list name?

It would apply to a list /after/ that list was expanded. Thus, expanding the list needs to happen /before/ any of those checks.

Strange, I am not sure where the list expansion does happen. I don't think bug 872041 changed the check on what is a valid recipient. But the bug does exist. I'll check it out.

OK, for now I have at least identified that reverting bug 68784 (moving the checkValidEmailAddress back to where it was) fixes the problem.

So DetermineHTMLAction calls checkAndPopulateRecipients and that expands mailinglists per the description at http://mxr.mozilla.org/comm-central/source/mailnews/compose/public/nsIMsgCompose.idl#148 . So I propose moving the expansion to sooner, before the checkValidEmailAddress. It can also reduce some code duplication.

Comment on attachment 751209 [details] [diff] [review] patch Review of attachment 751209 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, with some nits ::: mail/components/compose/content/MsgComposeCommands.js @@ +4031,5 @@ > +/** > + * Check what to do with HTML message according to what preference we have > + * stored for the recipients. > + * > + * @param convertible A nsIMsgCompConvertible constant describing An @@ +4046,5 @@ > var preferFormat; > > //Check the address book for the HTML property for each recipient > + let noHtmlRecipients = expandRecipients(true); > + if (noHtmlRecipients === false) { if (!noHtmlRecipients with the changes suggested below @@ +4059,2 @@ > > if (noHtmlRecipients != "" || noHtmlnewsgroups != "") while here if (noHtmlRecipients || noHtmlnewsgroups) @@ +4061,5 @@ > { > if (convertible == nsIMsgCompConvertible.Plain) > return nsIMsgCompSendFormat.PlainText; > > if (noHtmlnewsgroups == "") if (!noHtmlnewsgroups) @@ +4084,5 @@ > } > return nsIMsgCompSendFormat.AskUser; > } > else > return nsIMsgCompSendFormat.HTML; while here, remove the else. (no else after returns) @@ +4095,5 @@ > + * Expands mailinglists found in the recipient fields. > + * > + * @param aGetNonHtmlRecipients If this is set to true, the function returns > + * the list of recipients that prefer other format > + * than HTML. add @return description @@ +4101,5 @@ > +function expandRecipients(aGetNonHtmlRecipients) > +{ > + let obj; > + try { > + obj = new Object; you can move the let obj declaration to here. personally i also prefer new Object(); @@ +4106,5 @@ > + gMsgCompose.checkAndPopulateRecipients(true, aGetNonHtmlRecipients, obj); > + return obj.value; > + } catch(e) { > + Components.utils.reportError("gMsgCompose.checkAndPopulateRecipients failed: " + e); > + return false; null, i guess @@ +4109,5 @@ > + Components.utils.reportError("gMsgCompose.checkAndPopulateRecipients failed: " + e); > + return false; > + } > + > + return true; unreachable no?

Thanks, I have reworked the return values of expandRecipients() a bit. Can you please look at it?

Comment on attachment 751456 [details] [diff] [review] patch v2 Review of attachment 751456 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/components/compose/content/MsgComposeCommands.js @@ +4098,5 @@ > + * prefer other format than HTML is requested. > + * @return If aGetNonHtmlRecipients == true, then returns list of non-HTML > + * recipients. If it fails, returns null. > + * If aGetNonHtmlRecipients == false, then returns true or false > + * depending on whether the expansion of mailinglists succeeded. Grr, this mixing of return values sure is ugly, like the whole checkAndPopulateRecipients method. Could we maybe don't duplicate the ugliness and have one method for expanding and one for getting nonhtml recipients? @@ +4110,5 @@ > + return obj.value; > + > + return true; > + } catch(e) { > + Components.utils.reportError("gMsgCompose.checkAndPopulateRecipients failed: " + e); i know it was there before but is there really a reason for the try-catch? wouldn't it sooner or later have to go through successful expansion anyway to produce anything useful?

(In reply to Magnus Melin from comment #16) > Grr, this mixing of return values sure is ugly, like the whole > checkAndPopulateRecipients method. > > Could we maybe don't duplicate the ugliness and have one method for > expanding and one for getting nonhtml recipients? Method in nsIMsgCompose.idl ? > > @@ +4110,5 @@ > > + return obj.value; > > + > > + return true; > > + } catch(e) { > > + Components.utils.reportError("gMsgCompose.checkAndPopulateRecipients failed: " + e); > > i know it was there before but is there really a reason for the try-catch? > wouldn't it sooner or later have to go through successful expansion anyway > to produce anything useful? But it seems the function can fail. So why is the try-catch bad? What is the other alternative?

(In reply to :aceman from comment #17) > (In reply to Magnus Melin from comment #16) > > Grr, this mixing of return values sure is ugly, like the whole > > checkAndPopulateRecipients method. > > > > Could we maybe don't duplicate the ugliness and have one method for > > expanding and one for getting nonhtml recipients? > Method in nsIMsgCompose.idl ? Well, at least your helper js functions. Probably not a bad idea to split out CheckAndPopulateRecipients to not do multiple things, but that's more work for not so high reward. > > @@ +4110,5 @@ > > > + return obj.value; > > > + > > > + return true; > > > + } catch(e) { > > > + Components.utils.reportError("gMsgCompose.checkAndPopulateRecipients failed: " + e); > > > > i know it was there before but is there really a reason for the try-catch? > > wouldn't it sooner or later have to go through successful expansion anyway > > to produce anything useful? > But it seems the function can fail. So why is the try-catch bad? What is the > other alternative? Most things can fail, but if it's not an "expected exception" it just unnecessary - and things would fail later anyways - (e.g. the mailing list wasn't expanded). You can keep it if you think that's not the case.

Comment on attachment 751785 [details] [diff] [review] patch v3 Review of attachment 751785 [details] [diff] [review]: ----------------------------------------------------------------- Didn't test it, but looks good to me. r=mkmelin ::: mail/components/compose/content/MsgComposeCommands.js @@ +2684,5 @@ > msgType == nsIMsgCompDeliverMode.Later || > msgType == nsIMsgCompDeliverMode.Background; > if (sending) > { > + expandRecipients(false); no arg for this now @@ +4100,5 @@ > + gMsgCompose.checkAndPopulateRecipients(true, false, dummyObj); > +} > + > +/** > + * Returns recipients that prefer other message format than HTML. that prefer to get messages in plain text.

Ok, thanks.

Herm, rsx11m, are you able to test the patch? Or please comment after it lands in a TB nightly.

Be happy to test the patch. Let me know when it is available as part of the nightly build, or send a copy of the modified file, whichever is easier for you.

(In reply to Herm from comment #23) > Be happy to test the patch. Let me know when it is available as part of the > nightly build, or send a copy of the modified file, whichever is easier for > you. Intended to say, or let me know how to apply the patch. Hit send a little too quickly.

Herm, tinderbox builds should be available in 90-120 minutes, you could test those (may be easier than creating your own build or hacking a nightly one).

Confirmed. Bug resolved. Mail list function restored. Tinderbox build Index of /pub/mozilla.org/thunderbird/tinderbox-builds/comm-central-win32/1369083404 for Win32.

Great, thanks for checking. aceman, can you please nominate this for comm-aurora?

Comment on attachment 751798 [details] [diff] [review] patch v4 Thanks for testing. [Approval Request Comment] Regression caused by (bug #): bug 68784 User impact if declined: sending mailinglist (group in addressbook) not working Testing completed (on c-c, etc.): Tinderbox build Index of /pub/mozilla.org/thunderbird/tinderbox-builds/comm-central-win32/1369083404 Risk to taking this patch (and alternatives if risky): unknown, maybe the HTML convertibility could be affected by the patch

Using Thunderbird 17.0.7 on OS X, this bug is NOT fixed. It still insists on list@host, instead of expanding the addresses within the list. I use address lists only about once a month and this is the first time it happened.

That must be a different problem because the one fixed here was only introduced in TB23. You may have similar symptoms on TB17 but caused by a different problem. Please open a separate bug for it.

Since posting the original complaint (#31 above, yesterday) I have been able to test Thunderbird on other systems, in all cases using the current release, 17.0.7. The (very) brief tests show that Linux (on Ubuntu) and Windows (W 7) both work properly. So the fault seems to be only in the Mac OS X version. However these tests should be verified.

I wonder if it involves the length of the address list. Thunderbird (Mac 24.3.0) failed today (15 Fen 14) with a real-world list about 12 addresses, but worked with a simple test of 3.

No, 12 addresses should be fine. However this bug is marked fixed and there were no reports for half a year. Your problem is probably a different bug, please open a new bug. Thanks.

This bug appears to be back in 31.1. I had trouble that seems to match this bug and others appear to as well: http://forums.mozillazine.org/viewtopic.php?f=39&t=2865353 Should another bug be opened? A quick search didn't show any, but I may have missed one.

First make sure you are on TB31.1.1. See bug 1060901. Please do not reopen this one.

The behavior I mention below (observed in Thunderbird 38.7.0 of 14 March 2016, as installed on my Ubuntu 14.04LTS Linux system) might be not a bug but merely "how it is". Also, it may be a consequence of bug 1060901 mentioned in previous comment, 37. Anyhow, following guidelines in <http://kb.mozillazine.org/Command_line_arguments_(Thunderbird)> for creating an email at the command line, I entered thunderbird -compose "to='ttestgroup',subject='test7',body='body7'" where ttestgroup is a mailing list group with three email addresses on it, a nickname "ttest group", and a description "ttest group w/ 3 me". When I press return a Thunderbird message box labeled "Write: test7" pops up with subject=test7, body=body7, and the To: field showing as ttestgroup <>. (Also, an irritating "(process:2531): GLib-CRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed" message appears in the terminal where the command was entered.) When I click send, a thud sounds, and boxed message appears: "ttestgroup is not a valid e-mail address because it is not of the form user@host. You must correct it before sending the e-mail." On the other hand, the "Write: test7" message box popped up by the following command sends ok (but still gets GLib-CRITICAL message): thunderbird -compose "to='ttestgroup <ttest group w/ 3 me>',subject='test7',body='body7'" For a different mailing list called ttestgroupN with blank nickname and blank description and three email addresses, the first of the following commands gets a "ttestgroupN is not a valid e-mail address ..." message upon clicking Send, and the second one does not (ie the second form sends ok): thunderbird -compose "to='ttestgroupN',subject='test7',body='body7'" thunderbird -compose "to='ttestgroupN <ttestgroupN>',subject='test7',body='body7'"