Closed Bug 1336785 Opened 3 years ago Closed Last month

Problem with multiple consecutive commas in the To field

Categories

(MailNews Core :: MIME, defect)

defect
Not set

Tracking

(thunderbird_esr6869+ fixed, thunderbird70 fixed, thunderbird71 fixed)

RESOLVED FIXED
Thunderbird 71.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird70 --- fixed
thunderbird71 --- fixed

People

(Reporter: myaddons, Assigned: mkmelin)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

In the To field multiple recipients are separated by a comma. However, if there are multiple consecutive commas, then Thunderbird gets confused.

The problem can easily be the triggered, if the email addresses come from an external source, e.g. Microsoft Excel, LibreOffice Calc or any plain text document containing empty cells / rows.

Example plain text document:
info1@example.com
info2@example.com

info4@example.com

If you copy the contents of the example plain text document into the To field of Thunderbird, the line breaks are replaced by commas and it will be (correctly) shown as:
info1@example.com,info2@example.com,,info4@example.com

However, the multiple consecutive commas are then (miss-)parsed by Thunderbird internally. In the source code of the actual outgoing email the To field is mistakenly shown as:
To: info1@example.com, info2@example.com, ",info4"@example.com

As you can see the last recipient is now ",info4"@example.com, which is not correct anymore. It should still be info4@example.com!

I've tested quite a few versions of Thunderbird and the last working version was Thunderbird 24. The problem affects Thunderbird 31, 38, 45 and 52.0b1 and 54.0a1. So I think it might be related to the changes by JSMime.

It also looks like Bug 1280811 and Bug 1142428 might be related to this problem.
Keywords: regression
Version: 47 Branch → 45 Branch
You're dead right, since comes from JSMime which landed on TB 31 in bug 959309. It left us with some more comma problems like bug 1175900.
Component: Message Compose Window → MIME
Product: Thunderbird → MailNews Core
See Also: → 1175900
Version: 45 Branch → Trunk

This bug is still affecting Thunderbird 60.8.0 and Thunderbird 68.0.

Maybe Paul is interested in this one.

Flags: needinfo?(paul)

I see where this goes wrong.

Assignee: nobody → mkmelin+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9090343 - Flags: review?(jorgk)
Flags: needinfo?(paul)
Comment on attachment 9090343 [details] [diff] [review]
bug1336785_multi_comma.patch

Code is looking good. I haven't tried it since the test seems to be enough proof.
Attachment #9090343 - Flags: review?(jorgk) → review+
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 71.0
Attachment #9090343 - Flags: approval-comm-esr68?
Attachment #9090343 - Flags: approval-comm-beta+

Magnus, can you fix bug 1059988 in a similar way? I didn't want to comment there since it's a hornets nest.

Flags: needinfo?(mkmelin+mozilla)

And what about bug 1526804 while you're here?

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/10c8b6ffbd1e
account for multiple commas when parsing email addresses. r=jorgk

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Blocks: 1526804

Yeah, patch for bug 1059988 submitted.

Flags: needinfo?(mkmelin+mozilla)

Followup

Attachment #9091354 - Flags: review?(jorgk)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 9091354 [details] [diff] [review]
bug1336785_recipients_split.followup.patch

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

This works, but there's a question below.

::: mailnews/mime/src/mimeJSComponents.js
@@ +407,5 @@
>          addr = aDisplay.substr(0, comma);
>          aDisplay = aDisplay.substr(comma + 1);
> +
> +        // Make sure we don't have any "empty addresses" (multiple commas).
> +        comma = -1;

Any reason we don't start from 0 and use `comma` instead of `comma + 1`?

Either way works.

Attachment #9091354 - Attachment is obsolete: true
Attachment #9091354 - Flags: review?(jorgk)
Attachment #9091366 - Flags: review?(jorgk)
Comment on attachment 9091366 [details] [diff] [review]
bug1336785_recipients_split.followup.patch

Yes, but I prefer people not scratching their heads over the -1 and +1 ;-)
Attachment #9091366 - Flags: review?(jorgk) → review+
Attachment #9091366 - Flags: approval-comm-esr68?
Attachment #9091366 - Flags: approval-comm-beta+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/e50cfd3a402b
follow-up for handling of multiple commas in entered address. r=jorgk DONTBUILD

Status: REOPENED → RESOLVED
Closed: 2 months agoLast month
Resolution: --- → FIXED
Attachment #9090343 - Flags: approval-comm-esr68? → approval-comm-esr68+
Attachment #9091366 - Flags: approval-comm-esr68? → approval-comm-esr68+

Thank you for fixing this problem! I can confirm it is solved in Thunderbird 68.1.1.

And try semicolons ;-) - Bug 1059988.

You need to log in before you can comment on or make changes to this bug.