Closed Bug 1619057 Opened 5 years ago Closed 5 years ago

Mail Extension compose.onBeforeSend returning a mailing list fails

Categories

(Thunderbird :: Add-Ons: Extensions API, defect)

defect
Not set
normal

Tracking

(thunderbird_esr68 unaffected, thunderbird75 fixed, thunderbird76 fixed)

RESOLVED FIXED
Thunderbird 76.0
Tracking Status
thunderbird_esr68 --- unaffected
thunderbird75 --- fixed
thunderbird76 --- fixed

People

(Reporter: dave, Assigned: darktrojan)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:73.0) Gecko/20100101 Firefox/73.0

Steps to reproduce:

As mentioned in topicbox forum:
https://thunderbird.topicbox.com/groups/addons/T5d6979f2417f424c-M03013aee0a804913c76604c3/compose-onbeforesend-returned-object-format

Summary:
Using compose.onBeforeSend, not cancelling the send, and returning a mailing list in the [details] object fails "syntax error in parameters or arguments"
Screenshot:
https://revad.github.io/shared/Change_lint_to_BCC_Screenshot_2020-02-26_13-57-58.png

Object returned was
New obj:{"details":{"to":[],"cc":[],"bcc":["List_of_2_valid <List_of_2_valid>"],"replyTo":[],"followupTo":[],"newsgroups":[],"subject":"Test 2"},"cancel":false}

It doesn't make any difference whether you move the object to BCC or not.
(I also tried returning an object-formatted mailing list - that failed too.)

Addon I'm testing:
https://github.com/revad/limit_non-BCC_recipients

Blocks: webext-tb
Component: Untriaged → Add-Ons: Extensions API

I think I have found the problem. Mailing lists are not expanded before the onBeforeSend event fires, but this needs to happen afterwards in case the event changes the recipient list. So Thunderbird is trying to send to List_of_2_valid@your.domain instead of to the members of the list.

Assignee: nobody → geoff
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Actually the same applies if there is any onBeforeSend listener, regardless of whether the listener changes anything.

As always, 1 line of fix, 150 lines of test. I've also tidied up a few loose edges in the same test.

Attachment #9134593 - Flags: review?(mkmelin+mozilla)
Attachment #9134593 - Flags: approval-comm-beta?
Comment on attachment 9134593 [details] [diff] [review] 1619057-expand-lists-after-onbeforesend-1.diff Review of attachment 9134593 [details] [diff] [review]: ----------------------------------------------------------------- I'm a bit surprised this is needed. Would have thought the expansion happens later automatically, and in the backend somewhere
Attachment #9134593 - Flags: review?(mkmelin+mozilla) → review+

Expansion happens before our event which accidentally cancels it out by calling Recipients2CompFields again (via GetComposeDetails).

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0dac4d205acc
Expand mailing lists after onBeforeSend event fires. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 76.0
Comment on attachment 9134593 [details] [diff] [review] 1619057-expand-lists-after-onbeforesend-1.diff [Triage Comment]
Attachment #9134593 - Flags: approval-comm-beta? → approval-comm-beta+

This was fixed, but it's happening again in 76.0b1 and 77

Anything special about it? I suspect bug 1629103 but I can't reproduce so far.

I'm repeating the same test with the same list ("List_of_2_valid") as in the OP.
I return this from compose.onBeforeSend :
Returned compose object:{"cancel":false}
The result is exactly as in the screenshot in the OP.
The list contains, as its name suggests, 2 valid addresses.

I was using my addon:
https://addons.thunderbird.net/en-US/thunderbird/addon/limit-non-bcc-recipients/

I set the non-BCC limit in my prefs to 1 and the list contains 2 recipients so I get a warning.
If I select 'Send anyway' I get the error.
But if I select 'Send anyway' and 'Change all to BCC' it works.

The error happens when there is no 'details' key in the returned object, only a 'cancel' key - as in my comment 12. If I set ['details'] = {} it works.

I used to pass the original unchanged details back, adding the 'cancel' key - as in comment 1. But when 'body' 'plainTextBody' and 'isPlainText' was added to the API that stopped working (some message about 'you can't have both plaintext and html' IIRC). I fixed that by only passing changed fields.

So the 'details' key is mandatory. Or TB should treat its absence as 'no changes'.

I think I can see what that problem is.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9141925 - Flags: review?(mkmelin+mozilla)
Attachment #9141925 - Flags: approval-comm-beta?
Comment on attachment 9141925 [details] [diff] [review] 1619057-expand-lists-after-onbeforesend-again-1.diff Review of attachment 9141925 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, r=mkmelin
Attachment #9141925 - Flags: review?(mkmelin+mozilla) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/cffb0a855c7b
Expand mailing lists even if browser.compose.onBeforeSend listener returns nothing. r=mkmelin

Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Comment on attachment 9141925 [details] [diff] [review] 1619057-expand-lists-after-onbeforesend-again-1.diff approved for beta
Attachment #9141925 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: