1296535 - Cannot send message to distribution list

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Mark Schuster]

Attachment: Bug_Thunderbird_distribution_Lists.JPG

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

I have several distribution lists all under 30 recipients and when choosing a list (it doesn't matter which - they ALL do it), the "Send" tab is grayed out.  Neither "To:, :Cc:" nor "Bcc:" will accept a distribution list to send a message.

I can to each member by placing their address in the "To:" field.


Actual results:

Nothing can happen as the "Send" tab is grayed.


Expected results:

My distribution lists have not changed from 49.0a1 to 51.0a1.

Comment 1

[Mark Schuster]

I also tried re-creating the Lima Ambulance list from scratch on 51.0a1 and it too failed.  It doesn't matter if I send to "To:" or, "Cc:" or, "Bcc:" individually none work.

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks for reporting this. And thanks for using Daily, which can have some unexpected bugs ;-)

Note: TB 51. Aceman, we broke this in bug 1290733. We didn't think of mailing lists.

Comment 3

[:aceman]

Yeah. Expanding lists and then checking each recipient may be slow again.
But the previous method also didn't expand recipients, it just saw if anything was input. So do we again just check for a non-empty input (but not via msgCompFields)? Then expansion of ML and checking of @ in recipients is still done upon clicking Send.

Comment 4

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #3)
> But the previous method also didn't expand recipients, it just saw if
> anything was input. So do we again just check for a non-empty input (but not
> via msgCompFields)?
Sigh, I got used to the "Send" only becoming available after typing at least "a@b".

Is there no better way to detect mailing lists? List seem to show up as
  list <list>.

Comment 5

[:aceman]

https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#4785

I wouldn't want to reimplement that logic in JS again. Can we some of that existing code from the frontend?

Comment 6

[:aceman]

But I'd also like to avoid some expensive calls into nsMsgCompose and having to pass it all the input recipients first.

Comment 7

[:aceman]

Maybe we could use https://dxr.mozilla.org/comm-central/source/mailnews/addrbook/public/nsIAbManager.idl#158 on the input string?

Comment 8

[:aceman]

Attachment: patch

Maybe like this.

Comment 9

[Jorg K (CEST = GMT+2)]

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

Review of attachment 8783202 [details] [diff] [review]:
-----------------------------------------------------------------

How expensive is this?

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2966,5 @@
>      let popupValue = awGetPopupElement(row).value;
>      let inputValue = awGetInputElement(row).value.trim();
> +    if ((mailTypes.includes(popupValue) &&
> +        (isValidAddress(inputValue) ||
> +         MailServices.ab.mailListNameExists(inputValue.replace(/ <.*>/, "")))) ||

Looks reasonable. From past experience I'd be careful with the whitespace, so cater for zero to more than one space.

Comment 10

[:aceman]

On an extreme case of 1000 recipients where all of them are invalid strings (no @ and not a valid ML name) the loop takes 0.1-0.2 secs.

OK, I made the regexp '.replace(/ *<.*>/, "")'. It covers name formats of "list <anything>", "list<anything>" and even "list".

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: Added check for mailing list

Changed the regexp as discussed to / *<.*>/

I tested it and it seemed to work ;-)

Comment 12

[Jorg K (CEST = GMT+2)]

For those watching the bug: Aceman told me that he will be adding a test so this doesn't regress easily. Thanks, Aceman!

Comment 13

[:aceman]

Attachment: Added check for mailing list + test

Added the test. Can you imagine any other case we need to check? The test fails without the patch.

Comment 14

[Jorg K (CEST = GMT+2)]

Comment on attachment 8783357 [details] [diff] [review]
Added check for mailing list + test

Review of attachment 8783357 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. I didn't run it, but I suppose you did. No, I can't imagine more test cases. Well, unless you want to check trimming and such, so type
|   |
|  a@b  |
|  newsgroup  |
|list<list>|
|  list  <list>  |
etc. I tried those manually and I inspected the code to make sure we trim properly.

Can you do me a favour? Land this after bug 1296883 so I can see that the X tests clear again.

Comment 15

[:aceman]

OK, pushed also with some spaces to test the trimming.
https://hg.mozilla.org/comm-central/rev/93fae187ff9b3dbc56e53c89388b6e2f36fde00b

Comment 16

[Jorg K (CEST = GMT+2)]

Note, the Mozmill failures appearing in the push are not due to this bug, but due to the landing of bug 1265800. I'll raise a new bug for that.

Comment 17

[Mark Schuster]

I don't know if this has been included in the release 51.0a1 (2016-08-22) but in this update, sending to ML still does not work.

Also, second problem: I wanted to check the Release Notes (Settings/Help/Release Notes) to see if the patch had been applied but viewing the Release Notes returns the following web page: https://www.mozilla.org/en-US/thunderbird/nightly/releasenotes/?uri=/thunderbird/releasenotes/&locale=en-US&version=51.0a1&os=WINNT&buildid=20160822030425

Comment 18

[Jorg K (CEST = GMT+2)]

The bug got landed in the afternoon, 51.0a1 (2016-08-22) was built at noon. Try the Daily of 23rd.

There are no release notes for Daily versions.

Comment 19

[Mark Schuster]

Roger that.  Thanks for what you do!

Comment 20

[Mark Schuster]

I have to say something that I may have said before and will say again, I'm sure... I have to thank the men and women who spend countless hours of their own time, and I'm sure at the same pay as those of us who volunteer our time, as first responders, to help others.  Without their dedication and love of accomplishment when they finally squish the bug, we would not have these great products that increase our productivity, save us time and that we may take for granted.  My hat is off to you all and a special thanks to Jorg K and :aceman for your efforts in correcting this problem. 

Job well done!

Comment 21

[Jorg K (CEST = GMT+2)]

Thunderbird is maintained by overworked and unpaid(!) volunteers only. We accept donations:
https://donate.mozilla.org/en-US/thunderbird/about/

The reasons are given on the TB start page:
https://www.mozilla.org/en-US/thunderbird/release/start/

Comment 22

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #18)
> Try the Daily of 23rd.
Works now in this version.

Comment 23

[Mark Schuster]

I put my money where my mouth is... Done.  ;-)

Comment 24

[Craig L]

This bug seems to reappear in version 52.0 and 52.1.  I am unable to send message to distribution list unless I also add a single e-mail address in addition to the distribution list.

Comment 25

[:aceman]

What is your OS? Is that list in the Thunderbird internal addressbook or in Mac OS X addressbook or in Outlook addressbook?

Comment 26

[Craig L]

I am using Win10 Pro.  My distribution list is in the internal Thunderbird address book.  I do not use Mac OS or Outlook.  I have only used Thunderbird and the internal address book for years.

Comment 27

[:aceman]

You say the problem reappeared. In which TB versions did it work for you?

Comment 28

[Jorg K (CEST = GMT+2)]

Note that the reporter opened the exact same bug 1362130.

Comment 29

[Craig L]

Yes, that was my post.  I will continue this duscussion on but 1362130.  I started a new one because I didn't know if you would respond to a posting to a bug that was marked fixed.