+++ This bug was initially created as a clone of Bug #1663041 +++

STR

1. compose, and paste the following list of 5 comma-separated addresses into To-field, with one erroneous address in the middle (e.g. on German keyboard, if you don't hold AltGr long enough and press q key where the @ is third-level character, you'll just get q):
   john <john@example.com>, error <fooqbar.com>, jane <jane@example.com>, foo <foo@bar.com>, bar@baz.com
2. press Enter and count resulting pills

Actual result:

• 1 address lost, jane deleted! 4 out of 5 remain. Cunningly, only one good address after the erroneous address gets deleted, others remain. You may never notice that one is missing somewhere in the middle.

Expected result:

• Please don't lose pills when converting csv

Something's wrong here. Just having an error pill shouldn't eat random pills...
Can we please find the problem here and make sure that an erroneous address doesn't cause dataloss.

This goes through recipientAddPill() which calls MailServices.headerParser.makeFromDisplayAddress()

https://searchfox.org/comm-central/rev/421c8e4ca45dee96a86a73bb00cec4e7552f4f93/mailnews/mime/src/MimeJSComponents.jsm#401
_makeSingleAddress will get sent input of "error <fooqbar.com>, jane <jane@example.com>"

Basically every part is working as it should, and ultimately it's our internal maillist syntax which is to blame :/
Not sure if this is easily solvable. Luckily I don't think it's a common thing for people to enter "foo <foo@example.com>" type addresses.

(In reply to Magnus Melin [:mkmelin] from comment #1)

Basically every part is working as it should, and ultimately it's our internal maillist syntax which is to blame :/
Not sure if this is easily solvable. Luckily I don't think it's a common thing for people to enter "foo <foo@example.com>" type addresses.

Well, if every part was working as it should, this bug should not happen. Clearly our algorithm is wrong whichever way we look at it. I understand that "error <fooqbar.com>" could be mistaken as a mailing list, but that's no reason to dump the next address, which is complete and correct. Looking at the syntax of the CSV input string, "error <fooqbar.com>, jane <jane@example.com>" , there's no doubt syntactically that these are intended as two comma-separated addresses, so we're supposed to split at the comma first and then look at the two parts.

If we took the whole thing as one address, we'd have to consider the left side ("error <fooqbar.com>, jane") as display name, which we can't because it has unquoted angle brackets. Furthermore, at least we should create an error pill which has the full original string, instead of randomly cutting out an entire correct address just because the previous one is lacking an @.

I've tried to explain that the risk of accidentally typing another character for @ is much higher on keyboard layouts where @ is a third-level key requiring modifier keys to be pressed.

(In reply to Thomas D. (:thomas8) from comment #2)

no reason to dump the next address, which is complete and correct. Looking at the syntax of the CSV input string, "error <fooqbar.com>, jane <jane@example.com>" , there's no doubt syntactically that these are intended as two comma-separated addresses, so we're supposed to split at the comma first and then look at the two parts.

We can't do that, at least easily. That's why I blame the maillist syntax.
After we encounter an address, we must discard everything after it, or we expose users to odd attack scenarios (e.g. bug 1526456).

I think bug 1674650 would have fixed this. Please retest with today's nightly and dupe it if resolved.

I tested this and there's no more dataloss.
Bug 1674650 fixed it.