Closed Bug 1002024 Opened 9 years ago Closed 8 years ago

Sending separate invitations to attendees onlys send email for first invited person

Categories

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect, P1)

Lightning 3.2
x86_64
Windows 8.1
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aryx, Assigned: MakeMyDay)

References

Details

Attachments

(2 files, 1 obsolete file)

Daily 20140427 with latest Lightning 3.3a1

As discovered by rb in bug 544169, the separate email invitation per attendee only sends to the first person added to the attendee list of a new event.
Error message:

[JavaScript Error: "[Exception... "[JavaScript Error: "document.getElementById(...) is null" {file: "chrome://lightning/content/imip-bar.js" line: 138}]'[JavaScript Error: "document.getElementById(...) is null" {file: "chrome://lightning/content/imip-bar.js" line: 138}]' when calling method: [calIOperationListener::onOperationComplete]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///C:/Mozilla/Debugging/Thunderbird/Profiles/unstable/central/Test-central-en/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calCompositeCalendar.js :: calCompositeGetListenerHelper.prototype.onOperationComplete :: line 520"  data: yes]"]
This should have been fixed with bug 986852 - I'll take a look.
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Are you sure this error message is related to the reported issue? This indicates a problem with the implementation of bug 990009 under some conditions and is not related to the notification feature.
I didn't see the error this time, but if you check the Sent folder, there is only message: the one for the first attendee invited.
Did you only check the sent folder or also the inbox of the recipients?
Even as the error message from comment #1 is not related to this issue, it shouldn't occur. If you are able to reproduce that, can you please file a respective bug?
(In reply to MakeMyDay from comment #5)
> Did you only check the sent folder or also the inbox of the recipients?
I checked the inboxes of the recipients: If I let Tb send to all of them in one mail, the both get it, if it shall send separate mails, only the first one gets a mail.
Confirming the issue, even though it's not yet clear what's happening here. Unfortunately, I had a system crash yesterday, so it may take some days until my system is in shape again to proceed in investigation here.

The issue producing the error log message is a different one and I had already a fix for it before the crash. I will file a separate bug for that.
The mentioned bug in comment #8 related to the error message from comment #1 is bug 1002415.
This bug is a strange one. I identified the place, where thing are going wrong, but actually I have no idea, why this is happening.

_sendItem(...) at http://mxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#984 receives a value from aTransport.sendItems(...) (-> http://mxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js#132 ), which then is returned further.

While in aTransport.sendItems(...) TRUE is passed back, _sendItems(...) receives UNDEFINED. It seems there's a layer inbetween, that I'm not aware of - but if so, I have no idea why this worked before...

Does anybody has an idea about this?
Reason for this:

http://dxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#1001
  if (!_sendItem(sendToList, sendItem)) {

calls

_sendItem which calls

http://dxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#984
  return aTransport.sendItems(aSendToList.length, aSendToList, itipItem);

That calls http://dxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js#132
  return this._sendXpcomMail(aRecipients, aSubject, aBody, aItipItem);

were we get to http://dxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js#187
  switch (aItem.autoResponse) {

We pass that check at http://dxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js#188
  case (Components.interfaces.calIItipItem.USER): {

At the end of the method, there is a |return false|, only in |case (Components.interfaces.calIItipItem.AUTO): {| something different can be returned.

For this reason, the for loop at http://dxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#1001
  if (!_sendItem(sendToList, sendItem)) {
breaks.
Thank you for digging into this, but I think that's not the problem here. If aItem.autoResponse is of value Components.interfaces.calIItipItem.USER there is an indended fall through to the Components.interfaces.calIItipItem.AUTO section, which ends in return TRUE once a message is sent out.

To make this obvious, you can apply this logging patch and using verbose debug mode.
Blocks: TB31found
Despite the fact that this is marked as a blocker I doubt that it will delay the release of Lightning 3.3 that must ship together with Thunderbird 31. If you can't fix it I suggest to disable the new 'separate invitations' feature for the final 3.3 release instead of shipping a broken feature.
Version: Trunk → Lightning 3.2
I agree, this needs fixing or backing out on beta. I'm temporarily setting the milestone to 3.3 so I can easier track what still needs to be done before b2.
Target Milestone: --- → 3.3
Attached patch MakeSentItemReturnBooleanValue-V1.diff (obsolete) β€” β€” Splinter Review
Now, I guess what's going wrong here: Calling aTransport.sendItems(...) at http://mxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#1003 cannot return anything as the interface at http://mxr.mozilla.org/comm-central/source/calendar/base/public/calIItipTransport.idl#45 is defined to not to do so.

I have a simple patch to enable returning the result by changing the interface, but before asking for review I would like to test this. Unfortunately, the Calendar try builds seem to be broken since 19-Jun-2014 (as well as nightly builds of TB & Lightning).
Comment on attachment 8447720 [details] [diff] [review]
MakeSentItemReturnBooleanValue-V1.diff

I tested the patch successfully with a try build.
Attachment #8447720 - Flags: review?(philipp)
Comment on attachment 8447720 [details] [diff] [review]
MakeSentItemReturnBooleanValue-V1.diff

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

r=philipp and approval for aurora and beta. As beta 2 was just built, I'll probably include this manually so we can get some testing in.
Attachment #8447720 - Flags: review?(philipp)
Attachment #8447720 - Flags: review+
Attachment #8447720 - Flags: approval-calendar-beta+
Attachment #8447720 - Flags: approval-calendar-aurora+
Comment on attachment 8447720 [details] [diff] [review]
MakeSentItemReturnBooleanValue-V1.diff

Oh actually, we need to change the UUID of the interface since it changed. Thank you tree hooks for catching this!
Here is the same patch with uuid changed. I've pushed this to comm-beta so we can use this in Lightning 3.3b2 and up. 

https://hg.mozilla.org/releases/comm-beta/rev/56a7f0a73865

Keeping checkin-needed for aurora and central.
Attachment #8447720 - Attachment is obsolete: true
Attachment #8452469 - Flags: review+
Attachment #8452469 - Flags: approval-calendar-beta+
Attachment #8452469 - Flags: approval-calendar-aurora+
You need to log in before you can comment on or make changes to this bug.