463402 - Unwanted and confusing E-Mail notification dialogs ("Send out notification E-Mail now?" "Support Outlook 2000 and Outlook 2002/XP?")

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Mark Banner (:standard8)]

Attachment: The dialog

Dan sent me an invitation this morning (from a zimbra application I think). When I selected accept, I got a prompt:

E-Mail Notification
Would you like to send out notification E-Mail now?
[ ] Support Outlook 2000 and Outlook 2002/XP
       [cancel] [ok]

This raises several thoughts, that may end up covering separate bugs, but I wanted to raise them together first as some may be invalid or elsewhere:

1) I just selected Accept, why wouldn't I want to send out notification now?
2) Ok/cancel are wrong, it should be yes/no
3) How do I know if I need to support Outlook 2000 and Outlook 2002/XP? (this sounds like we're working around a different bug).

I also got the prompt when creating a new event and inviting people. Notify Attendees was checked (although I'd didn't actually look at it before I closed the event), so why do I need to be prompted there?

Comment 1

[Stefan Sitter [:ssitter]]

See Bug 377761 for information why the dialog was introduced.

Comment 2

[Daniel Boelzle [:dbo]]

Sad but true, people need to know what client the organizer or the attendees are using to judge on whether to use the compat OL2000 option.
One might tend to always use the OL2000 option, because it covers all later OL versions, too. The drawback is that it lacks a plain text body, and we can't send attachments with it (future).
IMO an alternative to get rid of the dialog would be to move the option to the prefs: I suspect most people will always use one or the other.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

Daniel, if the libmime jungle were to be revised, would there be more options?

Comment 4

[Daniel Boelzle [:dbo]]

Two questions on how to proceed here:
1. Should we always send mail without asking? Be aware that e.g. the organizer moving an event will automatically send updates to attendees then.
2. What shall happen to the OL compat mode option? -> Pref pane?

Comment 5

[Mark Banner (:standard8)]

One case I did see, which might want to be a separate bug, is the Lightning wants to send revised invites out if you click on Save and Close, even if nothing has changed. Give the save and close button is much bigger on the mac than the 'x', users are potentially more likely to hit that.

Comment 6

[Pete Riley]

> Should we always send mail without asking?

I'd like to see a distinction between (1) clicking the Accept/Decline buttons (automatically send response) and (2) editing an event (please ask us each time or use the checkbox "Send Attendees Invitations via Email" as Mark suggested).

For the second case, I don't want to spam all the attendees if I change only a couple of words in the Description field.

Comment 7

[Bryan Clark (DevTools PM) [:clarkbw]]

Mark: I think is a separate bug about being smart when closing an evet that hasn't changed.

When you make changes to an event I think we just need a quick confirmation dialog asking if you want to send out notification of the changes you made to all the attendees.  I don't usually advocate a dialog, however I think it's important in this process because we are going to send mail on your behalf.

Back to this bug... I think I was a little confused by this dialog.  When accepting an invitation does the person accepting normally send out an email notification?  Or was this just triggered by saving changes to the calendar event?  I don't think my accepting an invitation should cause a notification to be sent to the other attendees of the event.

The outlook question is difficult.  I think I would have moved toward defaulting to Outlook.  Not having the plaintext version isn't great, but I think it's easier to transition away from the compatible interface is easier than deciding what the clients your recipients have.  i.e. plaintext only people usually know who they are and complain when you don't send a plaintext mail; where everyone else doesn't notice.  And I suspect the cross section of calendar users and plain text only clients is much, much smaller than outlook + calendar users.  Getting the attachments to work with invitations would be the next bug to fix.

Comment 8

[Daniel Boelzle [:dbo]]

(In reply to comment #3)
> Daniel, if the libmime jungle were to be revised, would there be more options?
Hmm, maybe. IIRC I've seen some newer Outlook versions sending out a top-level multipart/alternative with enclosed text/plain, text/calendar parts, but I am not sure whether older Outlook versions swallow such. But worth a try anyway.

(In reply to comment #7)
> Back to this bug... I think I was a little confused by this dialog.  When
> accepting an invitation does the person accepting normally send out an email
> notification?  Or was this just triggered by saving changes to the calendar
> event?  I don't think my accepting an invitation should cause a notification to
> be sent to the other attendees of the event.
Yes, sending a confirmation is usual (RSVP). Your reply is sent to the organizer, not the other attendees.

> The outlook question is difficult.  I think I would have moved toward
> defaulting to Outlook.  Not having the plaintext version isn't great, but I
> think it's easier to transition away from the compatible interface is easier
> than deciding what the clients your recipients have.  i.e. plaintext only
> people usually know who they are and complain when you don't send a plaintext
> mail; where everyone else doesn't notice.  And I suspect the cross section of
> calendar users and plain text only clients is much, much smaller than outlook +
> calendar users.  Getting the attachments to work with invitations would be the
> next bug to fix.
I agree, though I'd prefer to keep a pref pane option. BTW: Google doesn't seem to send out iMIP that OL2000/2002 understands. Maybe we shouldn't care too much about it?

Comment 9

[Daniel Boelzle [:dbo]]

(In reply to comment #7)
> When you make changes to an event I think we just need a quick confirmation
> dialog asking if you want to send out notification of the changes you made to
> all the attendees.  I don't usually advocate a dialog, however I think it's
> important in this process because we are going to send mail on your behalf.

Reading comment #7 again, I think there's no real agreement yet. Mark and Pete vote for sending without reconfirmation (e.g. when accepting an invitation or having the "Notify attendees" checked).

Comment 11

[Philipp Kewisch [:Fallen][📱📆]]

Note from bug 430619: Users may also *never* want to send invitation mails, this should be taken into account when fixing this bug.

Comment 13

[Daniel Boelzle [:dbo]]

Sorry, but I won't find to drive this in the foreseeable future.

Comment 14

[patrick.abiven]

For a company usage, it could be great to enforce this option.
Why not supporting additional values for calendar.itip.compatSendMode ?
0 : default no but show dialog box
1 : default yes but show dialog box
2 : no and don't show dialog box
3 : yes and don't show dialog box
Regards
Patrick

Comment 15

[gabak]

who will fix it if it is assign to NOBODY ??
who makes lightning ?????
shame shame since 2008 with the same problem !!

Comment 16

[Bas van den Bosch]

http://support.microsoft.com/lifecycle/?p1=2557

outlook 2000 extended support had ended in 2009. We still use OL2000 at the office but to be honest I always hated the extra dialog too. So, I'm with patrick, let's drop the dialog and sent in outlook 2002/2003 by default.

Comment 17

[Philipp Kewisch [:Fallen][📱📆]]

(side note: Its funny that searching for "outlook 2000 user statistics" gives me Lightning as the second entry ;-)

This would be ok from my POV, if someone wants to whip up a patch go ahead. This is not a priority for the next beta though.

Comment 18

[Tristan Miller]

Is it not possible to determine which Outlook formats (if any) to support by looking at the User-Agent header of the e-mail containing the invitation?

Comment 19

[Philipp Kewisch [:Fallen][📱📆]]

Tristan, this is more about sending emails, not receiving them. Before sending an email we have no way of knowing if the receiving user will read it with outlook 2000 or not.


By the way, has anyone tested this with Outlook 2010 ? Do our invitations look ok there?

Comment 20

[Tristan Miller]

Well, presumably if the invitation was sent by a certain user agent, then it's a good bet the response will be read by the same user agent.  I don't know about you but I don't use separate e-mail clients for sending and receiving.

Comment 22

[Philipp Kewisch [:Fallen][📱📆]]

When fixing this bug, please also make note of the patch in bug 404050. There it is proposed to use a (hidden) pref to auto send notifications without this dialog.

Comment 23

[Andi]

It's a bit shocking to see the discussion (admittedly of a few issues) has continued since 2008 without resolution and update to the program.  What can we do to resolve the issues expediently?

I arrived at this bug on a Google search to find out why Lightning automatically re-notifies users without asking, each time an appointment is re-saved; so I'll give my point of view about that.

If a diary event belongs just to me, I can annotate it with small details as they arise, maybe correct a spelling mistake or reword a phrase for clarity.  I may have cause to do that a dozen times in preparation for an appointment.

If I add other participants, each small adjustment will send them a notification.  I may save several changes (maybe from invitee feedback) which I save during a day; and I will only want a re-notification to be sent out once, after all feedback is returned.  

It's kind of horrific to think that everybody is being e-mailed with the minutiae of my adjustments - re-notification has to be my judgement call.

Outlook 2000 (the only other program with which I can compare) asks me on each save if I want to send updates to all attendees.  Surely this is the appropriate behaviour, isn't it?

Comment 24

[monperrus]

Would love to see this fixed. Like Patrick's proposal:

calendar.itip.compatSendMode ?
0 : default no but show dialog box
1 : default yes but show dialog box
2 : no and don't show dialog box
3 : yes and don't show dialog box

Comment 25

[Charles]

So... where ion the heck can I find documentation on what the available options are for this config option? I googled till my fingers bled...

Comment 26

[Bas van den Bosch]

bug 377761
I noticed communigate mapi connector also has this option in the configuration-window so we aren't the only ones having problems with it.

Last week windows-xp extended support was ended. Outlook 2000 doesn't run on windows 7 so its usage will be declinging. Outlook 2002/xp on windows 7 has problems too. Perhaps it would be wise to make this a config option rather than a dialog?

Comment 27

[Charles]

Anyone?

I have some Thunderbird systems that do not prompt... apparently someone clicked the checkbox to not prompt again, and I can't for the life of me figure out how to bring that prompt back.

Comment 28

[Andi]

I don't know what in particular you want to have a prompt for, but try looking in Options | Advanced | Config Editor | (Confirm "Here by Dragons") | Search for "ask", and see if any of the options fit the bill.

Comment 29

[Charles]

?

I'm talking about the Prompt that is the subject of this bug. Not sure how that was unclear?

I have users who see NO prompt to send out these invite/updates...

Comment 30

[Andi]

Ok, sorry if my suggestion didn't help.

Comment 31

[Charles]

Sorry, maybe that sounded too harsh... but I have searched and searched the config editor for any setting that might control this, and the only one that seems like it would/should is this one, but no one seems to be able to tell me what are the available options...

Is it only 0 or 1?

There *has* to be a way to get this dialog back...

Comment 32

[Magnus Melin [:mkmelin]]

Source code woudl be your best friend.
http://mxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js#214
http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/lightning.js#59
 
So calendar.itip.compatSendMode 1 I guess

Comment 33

[Magnus Melin [:mkmelin]]

(In reply to Bas van den Bosch from comment #26)
> Last week windows-xp extended support was ended. Outlook 2000 doesn't run on
> windows 7 so its usage will be declinging. Outlook 2002/xp on windows 7 has
> problems too. Perhaps it would be wise to make this a config option rather
> than a dialog?

Do you mean it works on win7 or not?
Either way, personally I doubt those versions are very much used anymore and the dialog pref should probably be flipped.

Comment 34

[monperrus]

It seems that this issue is about two different things:

Disabling the pop-up:
"Send out notification E-Mail now?"

Disabling the option in the pop-up:
"Support Outlook 2000 and Outlook 2002/XP?"

Personally, I want to disable the pop-up. As far as I understand, this is not doable with calendar.itip.compatSendMode, calendar.itip.notify or calendar.itip.notify-replies.

How to disable the pop-up?

Comment 35

[mozilla-lightning-patch]

Attachment: notificatonDialog.patch

Hey there, it seems that after 6 years this might be a possible workaround. :)

Take a look at the attached patch for these files:
- http://mxr.mozilla.org/comm-central/source/calendar/itip/calItipEmailTransport.js
- http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/lightning.js

This patch was inspired by the proposal of "[e-mail]". I decided to keep
"calendar.itip.compatSendMode" and imipSendMail dialog options separated. The following 3
options can be set with the newly introduced "calendar.custom.sendReplyNotificationEmail":

  '0' -> Prevent dialog and do not send a notification mail

  '1' -> Prevent dialog and do send a notification mail

  '2' -> Show dialog (default, behavior unchanged)

Note: Case '1' will use the currently configured value of the existing 
"calendar.itip.compatSendMode".

With best regards

Amin Zoubaa

Comment 36

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8526955 [details] [diff] [review]
notificatonDialog.patch

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

Hi Amin, sorry for delaying this so long. You've done everything right, the patch is very clean and seems straightforward. I'd like to give the patch a test run before marking it r+, and I have one minor nit.

I probably won't get to testing this before Wednesday, maybe you can upload a new patch with the nits fixed in the meanwhile? If you have any questions, feel free to email me directly or visit me on irc.mozilla.org.

::: calendar/itip/calItipEmailTransport.js
@@ +203,4 @@
>                      break;
> +                }
> +                // show a dialog prompting user for action
> +                if (Preferences.get("calendar.imip.sendReplyNotificationEmail", 2) === 2) {

Instead of calling 	
Preferences.get("calendar.imip.sendReplyNotificationEmail", 2) multiple times, I'd suggest to use an intermediate variable.

Also, I don't think we need a strict compare here, == should be enough.

Comment 37

[mozilla-lightning-patch]

Attachment: notificatonDialog.v2.patch

Hi Philipp,

thanks for your feedback. I have updated my modifications and attached a new patch without the minor nits. Everything else remains as it was.

I hope the testing goes well and look forward to hearing from you :) 

best regards,

Amin Zoubaa

Comment 38

[[:MakeMyDay]]

I have had also a look at this patch. Codewise it looks good, but I would like to challenge the approach:

The option 0 in the patch doesn't really makes sense to me. It applies both on invitations and replies. While I see a valid use case for the latter one, I don't do so for the former. And if you don't want to send out replies, you would be forced to toggle the prefs to still be able to send out invitations. This would lead from one poor UX to another.

Instead of this approach, I suggest to remove the compat mode - if somebody in 2014 is still running Outlook 2000/XP, he/she might have much more problems than not being able to deal with Lightning invitations... That said, it is reasonable to me and would enables us to get rid of the dialog easily.

However, I would like to keep the choice for the user whether or not to send a reply - and not hidden in a pref. We can add this more elegant as different choices to the accept/decline buttons for invitations (now that we have the drowdown buttons) as well as to the event context menu in calendar view and the dropdown of the event summary dialog instead with sending as default option. As a plus, this would enable us to intercept the process earlier to prevent sending not only of email invitations to offer this option for all calendars with clientside scheduling management.

For invitation, I think we don't need any special handling, sending them out on hitting save&close is the relevant option here (optionally, we can it more obvious by turning the label to 'send' if there are attendees in this event).

Amin, Philipp what do you think?

It may sound to be more work than it will be - the implentation would be straight forward. If you needed further support to follow this approach, I'm willing to help. (This has been already on my list but with a low priority).

Comment 39

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8536572 [details] [diff] [review]
notificatonDialog.v2.patch

MakeMyDay, I'm always for getting rid of old code and I think you have a good grip on it. Do you think you could take over testing/reviews here?

Also looking forward to hear what Amin thinks.

Comment 40

[[:MakeMyDay]]

Sure, I will take care. But let's wait for Amin response.

Comment 41

[[:MakeMyDay]]

Comment on attachment 8536572 [details] [diff] [review]
notificatonDialog.v2.patch

Codewise, this looks good, but r- for the approach - sorry for this. Amin can you elaborate this patch into the direction of my above comment and rerequest review once done? If you need more guidance inbetween, please leave a comment here.

Comment 42

[[:MakeMyDay]]

Amin, are still working on this or willing to?

Comment 43

[Luke Jackson]

MakeMyDay - I think its safe to say Amin is no longer working on this. Are you able to take over from here?

If not, perhaps we should split this issue in two to try and get some progress. Issue #1: remove legacy Outlook 2000/XP compatibility mode - which should be relative uncontentious, and then Issue #2: allow email confirmations to be send always/never/prompt

Comment 44

[monperrus]

> If not, perhaps we should split this issue in two to try and get some progress.
+1

Comment 45

[[:MakeMyDay]]

I take this on my list. But please be patient a little bit - the release train for the upcoming major release 4.0 is already gone, so this can only be part of the next.

And please don't add such +1 comments, you can vote for the bug instead.

Comment 46

[monperrus]

MakeMyDay - would you post an update on this old bug?

Comment 47

[[:MakeMyDay]]

It is still work in progress. This requires large changes allover the application to be working reliably. Therefor it missed the train für 4.7.

I have to reorganize my work queue now that 4.7 was released. But I'm confident to land the first patches for this within the next couple of weeks (depends also whether regression in 4.7 show up with require immediate attention). So stay tuned.

Comment 48

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part1-Processing-V2.diff

Ok, this is the first and most important (WIP) patch of a series of patches to get this done. It covers the processing part. Without any of the other patches making use of it, it will not change the current behaviour.

As this is the foundation for everything else in this bug, I would appreciate some feedback. I'll still upload another patch making use of it for the imipbar to demonstrate the way it works.

This patch requires the patches from bug 1317064 and bug 1225784 to be applied (in that order).

Comment 49

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part2-ImipBar-V1.diff

This patch makes use of part 1 to offer the ability to send or not send a reply for the imipbar.

There would be part 3 and 4 to add the same options to the context menus in calendar views and the (summary) dialog.

This bug will have several string additions, so we will still need to decide whether we really want this to include in 5.4, but given we have an extended Aurora period this time, I think this is at least worth to consider.

Comment 50

[[:MakeMyDay]]

Comment on attachment 8812546 [details] [diff] [review]
RemoveSendNowDialog-Part1-Processing-V2.diff

Removing f? for now. This patch still has flaws, I'll upload another version.

Comment 51

[[:MakeMyDay]]

This bug missed the cut for string changes, so I'm removing the tracking for 5.4.

Comment 52

[monperrus]

Any hope for having "Support Outlook 2000 and Outlook 2002/XP" removed?

Comment 53

[monperrus]

Attachment: patch for removing "Support Outlook 2000 and Outlook 2002/XP?"

Here is a tentative patch for the second part of this bug related to the support of old versions of Outlook ("Support Outlook 2000 and Outlook 2002/XP?")

Comment 54

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8873995 [details] [diff] [review]
patch for removing "Support Outlook 2000 and Outlook 2002/XP?"

Sorry I haven't gotten around to this. I may have missed something from previous comments, but maybe it makes sense for MakeMyDay to review this one instead.

From a brief glance, I suspect there is some more work needed if this is the right way forward, e.g. removing strings and either not calling the prompt at all or correcting the wording.

Comment 55

[[:MakeMyDay]]

Comment on attachment 8873995 [details] [diff] [review]
patch for removing "Support Outlook 2000 and Outlook 2002/XP?"

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

Thanks for stepping up. I know this bug lives now for quite a while and had to be descoped for the last ESR, which might have been disappointing for people observing this bug. But I'm sure to get this bug fixed for the next ESR. 

This patch is a subset of the processing part patch for this bug, which is currently work in progress (effectively, I picked this up again two weeks ago). There is more to change also for the OL compatibility aspect of this bug, then your patch covers, as Philipp already suggested. But I prefer to leave this part of the mentioned patch, as I already covered this fully in may local version of the patch, so r- for this reason. Please don't take this a disregard of your participation.

If you're interested especially in supporting the progress of this bug, stay tuned, we will need fairly extentsive manually testing once the patches are ready to land as we don't have a good coverage with our automated tests for email invitations - so we will need you. If you want to work on other issues, look at [1] for good first bugs.

If you provide a patch then, please make sure you have incorparted an appropriate patch header including the author and a commit message mentioning the respective bug, a description of what the patch does. There will be bonus points, if you already add a suffix to the commit message mentioning the reviewer, so this doesn't need to be added by the sheriff when landing a patch for you (e.g. "Bug 123456 - This patch does some coo staff;r=(nick)name of the reviewer" see other bugs for examples). And, as you already learned, ask for review when uploading - otherwise there is a good chance your patch wouldn't be looked at for quite some time.

[1] https://www.joshmatthews.net/bugsahoy/?calendar=1

Comment 56

[monperrus]

> But I'm sure to get this bug fixed for the next ESR. 

Excellent! I look forward to it. Thanks.

Comment 57

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part1-Processing-V3.diff

Updated version of the patch, almost ready.

Comment 58

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part2-ImipBar-V2.diff

Updated version of the patch. As discussed on irc, I moved the tentative actions to a separate button, but made this behind a pref - the new 4 button style will now be the default, but can be switched back to the current 3 button style. Background: there will be additional options (like "send with comment" per button in (a hopefully not to far) future.

This patch requires part 1 to be applied before.

Comment 59

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part4-ContextMenus-V1.diff

Part 4 of this series, taking care of the context menus (still needs some polishing)

There are a lot of comments in the patch that gives you a good grip, especially since the current behaviour will slightly change.

Philipp, I mostly would appreciate your feedback on the large comment in calendar-ui-utils.js, where I described how this should be working.

Richard, I would like to get your attention mostly on the checkmark icon I used in the context menu and the different colors in case occurrence and master partstat are different. But of cause any other feedback is welcome.

This patch requires part 1 and 2 applied before. Part 3 is still work in progress, so you currently cannot make benefit from the new options from the event and summary dialog / event tab yet, but you can apply part 4 without part 3.

Comment 60

[[:MakeMyDay]]

One more thing: since part 1 contains an interface change, you will need a clobber build to get this working.

Comment 61

[Richard Marti (:Paenglab)]

I'll test this this weekend. MakeMyDay, please could you attach test invitations for the different cases?

Comment 62

[[:MakeMyDay]]

Thanks. For displaying the buttons for email ivitations you can simply use any invitation you haven't yet responded. If you haven't any at hand, you can e.g. take attachment 8924320 [details] (by doing the latter, you could also reproduce bug 1414608, btw).

Regarding the context menu, you can simply create a single and a recurring event and invite e.g. [e-mail] if you have made sure to assign an email identity to the calendar you use for that purpose. After saving & closing the events, the attendance item shows up in the context menu for these events in the calendar views or the today pane (the party, who's partstat you could change in this setup would be the organizer, but it works respectively for received invitations, where you are an attendee).

Comment 63

[Richard Marti (:Paenglab)]

Comment on attachment 8924297 [details] [diff] [review]
RemoveSendNowDialog-Part2-ImipBar-V2.diff

Patch 1 and 2 didn't apply cleanly. I had to rebase them. And with calendar.itip.separateInvitationButtons manually applied because of the extension prefs import bug, I saw the four buttons.

With the future plans, four buttons makes sense. r+

Comment 64

[Richard Marti (:Paenglab)]

Comment on attachment 8924300 [details] [diff] [review]
RemoveSendNowDialog-Part4-ContextMenus-V1.diff

ui-r- because the checkmark is too big in the context menu. Why don't you use menuitem type="checkbox"? With this, a system integrated appearance would be guaranteed. Or had you other things in mind?

Comment 65

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8924300 [details] [diff] [review]
RemoveSendNowDialog-Part4-ContextMenus-V1.diff

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

Wow, kudos for the elaborate description! I think this makes complete sense :) Minor comment on the remaining code, which you might have taken care of anyway:

::: calendar/base/content/calendar-common-sets.xul
@@ +215,3 @@
>              label="&calendar.context.attendance.menu.label;"
>              accesskey="&calendar.context.attendance.menu.accesskey;"
> +            oncommand="setContextPartstat(event.target.getAttribute('respvalue'), event.target.getAttribute('scope'), currentView().getSelectedItems({}), event.target.hasAttribute('respmode') ? { responseMode: Components.interfaces.calIItipItem[event.target.getAttribute('respmode')] } : null)"

This is starting to get pretty long, maybe we can put it into a function.

@@ +538,3 @@
>              label="&calendar.context.attendance.menu.label;"
>              accesskey="&calendar.context.attendance.menu.accesskey;"
> +            oncommand="setContextPartstat(event.target.getAttribute('respvalue'), event.target.getAttribute('scope'), getSelectedTasks(), event.target.hasAttribute('respmode') ? { responseMode: Components.interfaces.calIItipItem[event.target.getAttribute('respmode')] } : null)"

Same here.

Comment 66

[[:MakeMyDay]]

Finally, I think the patches are ready for review now. There are 6 patches with different scopes, which need to be applied in ascending order:

#1: processing

- adding support for processing in different response modes (AUTO, USER, NONE)
- removing compatibility mode for OL 2002 and earlier


#2: imipbar

- providing the option to accept/decline an invitation from mail view with or
  without notifying the organizer
- re-introcunding separate buttons for accept and tentative in invitation emails
  (having a pref to switch to 3 button mode, disabled by default)
  

#3: dialogs/tabs (summary and event dialog, event tab)

- added a toolbar to summary dialog if not read-only to trigger status updates with or without notifying
- displaying the calendar name in summary dialog (without an option to change the calendar)
- dynamic replacement of toolbar and menu items depending on existnace of attendees in event dialog/tab

- not included and something to follow-up: use separate icons in event dialog/tab for 'send and close'
  and 'send and save' toolbarbuttons (icons only needed)

#4: context menu (calendar view and today pane)

- providing the option to accept/decline invitations from context menu with/without sending a reply
- normalizing the strings to deal with invitations so they are aligned with what we use at other places,
  e.g. use 'accept' instead of 'I will attend' and reuse send/dontsend strings for all partstats

#5: delete, cut, copy & paste

- adds an option paste with or without notification if the popup to choose the target is prompted,
  otherwise fall back to pre-exsiting popup (=USER mode)
- explicitely falling back to pre-existing popup (=USER mode) when cutting or deleting an item

- not included and something to follow-up: find a way to combine the request for cutting and
  pasting when it comes to sending notifications - currently, there is no concept for a cutting to
  be related to a pasting within the app and only delete the item if pasting was successful


#6: drag 'n drop

- explicitely falling back to pre-existing popup (=USER mode) when droping an item


The patch from bug 1437405 should be applied before to have dnd working and also attachment 8955883 [details] [diff] [review] form bug 393084. Apart from that, the patches build on top of the already landed calUtils changes, but doesn't consider not yet landed changes. Part 1 involves an interface change, so you need to compile.

With the whole set applied, the linter and mozmill tests are passing locally.

Part 1 to 5 include string changes, so we should get them landed before the string freeze.

Comment 67

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part1-Processing-V4.diff

For the scope of this patch, see comment 66.

Comment 68

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part2-ImipBar-V2.diff

For the scope of this patch, see comment 66.

Comment 69

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part3-Dialogs-V1.diff

For the scope of this patch, see comment 66.

Comment 70

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part4-ContextMenus-V2.diff

For the scope of this patch, see comment 66.

Comment 71

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part5-CopyCutPaste-V1.diff

For the scope of this patch, see comment 66.

Comment 72

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part6-DragDrop-V1.diff

For the scope of this patch, see comment 66.

Comment 73

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8955884 [details] [diff] [review]
RemoveSendNowDialog-Part1-Processing-V4.diff

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

::: calendar/base/modules/calItipUtils.jsm
@@ +636,5 @@
>       * deleted and sends out appropriate iTIP messages.
> +     * @param {long}               aOpType        Type of operation - (e.g. ADD, MODIFY or DELETE)
> +     * @param {calIEvent|calITodo} aItem          The updated item
> +     * @param {calIEvent|calITodo} aOriginalItem  The original item
> +     * @param {JSObject}           aExtResponse   [optional] An object to provide additional

Please use js types here as far as possible, i.e. Number,Object

@@ +690,5 @@
>                  }
>              }
>          }
> +        // for backward compatibility, we assume USER mode if not set otherwise
> +        let ciii = Components.interfaces.calIItipItem;

We've been using cIII in other places and similar constructs.

@@ +990,5 @@
>       *
> +     * @param {calIItipItem} aItem            item to be sent
> +     * @param {String}       aMethod          iTIP method
> +     * @param {Array}        aRecipientsList  array of calIAttendee objects the message should be sent to
> +     * @param {JsObject}     aAutoResponse    JS object whether the transport should ask before sending

Object

Comment 74

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8955885 [details] [diff] [review]
RemoveSendNowDialog-Part2-ImipBar-V2.diff

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

::: calendar/lightning/content/imip-bar-overlay.xul
@@ +140,5 @@
>                  <menupopup id="imipAcceptDropdown">
>                    <menuitem id="imipAcceptButton_Accept"
>                              tooltiptext="&lightning.imipbar.btnAccept2.tooltiptext;"
>                              label="&lightning.imipbar.btnAccept.label;"
> +                            oncommand="ltnImipBar.executeAction('ACCEPTED', { responseMode: Components.interfaces.calIItipItem.AUTO });"/>

Ci

::: calendar/locales/en-US/chrome/lightning/lightning.dtd
@@ +52,5 @@
>  
>  <!-- iMIP Bar (meeting support) -->
>  <!ENTITY lightning.imipbar.btnAccept.label                                  "Accept">
>  <!ENTITY lightning.imipbar.btnAccept2.tooltiptext                           "Accept event invitation">
> +<!ENTITY lightning.imipbar.btnAcceptDontSend.label                          "Accept without sending">

Is it clear from the context without what is being sent? I can imagine it is hard to get this onto a button label

Comment 75

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8955886 [details] [diff] [review]
RemoveSendNowDialog-Part3-Dialogs-V1.diff

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

::: calendar/base/content/dialogs/calendar-summary-dialog.js
@@ +320,5 @@
> +        // we display a notification about the users partstat
> +        let partStat = window.attendee.participationStatus || "NEEDS-ACTION";
> +
> +        let msgStr;
> +        switch (partStat) {

Would be more compact to turn this into a map, i.e.

let strmap = {
  ACCEPTED: "itemAccepted",
  ...
}

::: calendar/locales/en-US/chrome/calendar/calendar-event-dialog.properties
@@ +457,5 @@
> +# LOCALIZATION NOTE (itemTentative) - this will be displayed as notification
> +# in the summary dialog if the user has accepted the invitation or the assigned
> +# task tentatively
> +# %1$S - either typeEvent or typeToDo
> +itemTentative=You have accepted this %1$S tentatively

Inserting event/todo here might cause some trouble if the translation is different, e.g.

Du hast diese |Aufgabe| azeptiert
Du hast diesen |Termin| Akzeptiert

(I realize that it is event invitation instead of just event, but I can still imagine it might be different in other locales. Would it be feasible to split these strings into event/task variants?

@@ +492,5 @@
> +# invitation
> +typeEvent=event invitation
> +
> +# LOCALIZATION NOTE (typeToDo) - argument to use in item* in case of a task
> +# assigned to the user by somebody else 

Whitespace

Comment 76

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8955887 [details] [diff] [review]
RemoveSendNowDialog-Part4-ContextMenus-V2.diff

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

::: calendar/base/content/calendar-common-sets.js
@@ +802,5 @@
>  /**
>   * Handler function to set up the item context menu, depending on the given
>   * items. Changes the delete menuitem to fit the passed items.
>   *
> + * @param  {nsIDOMEvent} aEvent   The DOM popupshowing event that is  triggered

nsIDOMEvent is going away, but DOMEvent is correct.

@@ +804,5 @@
>   * items. Changes the delete menuitem to fit the passed items.
>   *
> + * @param  {nsIDOMEvent} aEvent   The DOM popupshowing event that is  triggered
> + *                                  by opening the context menu
> + * @param  {Array}       aItems   An array of items (usually the selected items)

Either {Array.<calIItemBase>} or {calIItemBase[]} if you want to adhere to jsdoc :)

@@ +814,5 @@
>          let menuItem = document.getElementById(aMenuItemId);
>          if (menuItem) {
> +            menuItem.setAttribute(
> +                "label",
> +                cal.calGetString("calendar", "delete" + aItemType + "Label")

This is probably a good line to use a template string, but not necessary if you prefer normal strings.

::: calendar/base/content/calendar-item-editing.js
@@ +779,5 @@
>  
> +                let extResponse = null;
> +                if (aTarget.hasAttribute("respmode")) {
> +                    let mode = aTarget.getAttribute("respmode");
> +                    let itipMode = Components.interfaces.calIItipItem[mode];

Ci

::: calendar/base/content/calendar-ui-utils.js
@@ +797,5 @@
> +    /**
> +     * Provides the user's participation status for a provided item
> +     *
> +     * @param   {calEvent|calTodo}  aItem  The calendar item to inspect
> +     * @returns {Mixed}             The participation status string as per

jsdoc says {?String} is a nullable string.

Comment 77

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8955888 [details] [diff] [review]
RemoveSendNowDialog-Part5-CopyCutPaste-V1.diff

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

::: calendar/base/content/calendar-clipboard.js
@@ +196,5 @@
> +            // we only will need to ask whether to send notifications, if there
> +            // are attendees at all
> +            let withAttendees = items.filter((aItem) => {
> +                return aItem.getAttendees({}).length > 0;
> +            });

You can probably get this into one line

let withAttendees = items.filter(aItem => aItem.getAttendees({}).length > 0);

@@ +223,5 @@
> +                            destCal = aCal;
> +                            notify = false;
> +                        };
> +                        args.labelOk = cal.calGetString("calendar",
> +                                                        "pasteAndNotifyLabel");

If you can get these into a line of 100 please do. I didn't count though.

::: calendar/locales/en-US/chrome/calendar/calendar.properties
@@ +53,5 @@
>  exportPrompt=Which calendar do you want to export from?
>  pastePrompt=Which of your currently writable calendars do you want to paste into?
>  publishPrompt=Which calendar do you want to publish?
>  
> +notificationPrompt=Do you want to notify the participants of the pasted items?

Would the user know what the participants are being notified of here?

Comment 78

[Philipp Kewisch [:Fallen][📱📆]]

Generally, I am on the fence on using jsdoc types. On the one hand I can see how it makes sense, but we haven't been doing this very often and since the types are not checked the additional use is marginal.

Comment 79

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part2-ImipBar-V3.diff

I just noticed I uploaded an outdated patch for part 2 - sorry for the inconvenience.

Comment 80

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8957791 [details] [diff] [review]
RemoveSendNowDialog-Part2-ImipBar-V3.diff

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

::: calendar/locales/en-US/chrome/lightning/lightning.dtd
@@ +83,5 @@
>  <!ENTITY lightning.imipbar.btnUpdate.tooltiptext                            "Update event in calendar">
>  <!ENTITY lightning.imipbar.description                                      "This message contains an invitation to an event.">
>  
> +<!ENTITY lightning.imipbar.btnSend.label                                    "Send a response now">
> +<!ENTITY lightning.imipbar.btnSend.tooltiptext                              "Send out a response to the organizer">

"Send out" sounds a bit colloquial. Maybe just "Send a response to the organizer" ?

@@ +84,5 @@
>  <!ENTITY lightning.imipbar.description                                      "This message contains an invitation to an event.">
>  
> +<!ENTITY lightning.imipbar.btnSend.label                                    "Send a response now">
> +<!ENTITY lightning.imipbar.btnSend.tooltiptext                              "Send out a response to the organizer">
> +<!ENTITY lightning.imipbar.btnSendSeries.tooltiptext                        "Send out a reply for the entire series to the organizer">

You use response above, and reply here. Can we unify this? If not, then a localization note may be required, otherwise it may be translated wrong.

Comment 81

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part1-Processing-V5.diff

Updated patch with comments considered

Comment 82

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part2-ImipBar-V4.diff

Updated patch with comments considered.

Comment 83

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part3-Dialogs-V2.diff

Updated patch with comments considered.

Comment 84

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part4-ContextMenus-V3.diff

Updated patch with comments considered.

Comment 85

[[:MakeMyDay]]

Attachment: RemoveSendNowDialog-Part5-CopyCutPaste-V2.diff

Updated patch with comments considered. I have split up the notificationPrompt in a more fine grane approach referring to pasting a meeting or assigned event, so the context should be more clear why we offer to send a notification. However, It's hard to tell the user what exactly changed, we keep it a little vague here.

Comment 86

[[:MakeMyDay]]

The patches need to be applied in ascending order (part 1 to 6). Please land this before the merge since these have l10n changes. Thanks.

Comment 87

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/044240bd35e0
Remove E-Mail notification dialogs - Part 1: processing;r=philipp
https://hg.mozilla.org/comm-central/rev/5ae749bd1ed8
Remove E-Mail notification dialogs - Part 2: imip bar;r=philipp
https://hg.mozilla.org/comm-central/rev/f5138ae6a728
Remove E-Mail notification dialogs - Part 3: dialogs;r=philipp
https://hg.mozilla.org/comm-central/rev/f4e55986c496
Remove E-Mail notification dialogs - part 4: context menus;r=philipp
https://hg.mozilla.org/comm-central/rev/bdc3de3a59b4
Remove E-Mail notification dialogs - part 5: delete, copy, cut & paste;r=philipp
https://hg.mozilla.org/comm-central/rev/0c44042f9e67
Remove E-Mail notification dialogs - part 6: drag & drop;r=philipp