1572379 - accept/tentative/decline toolbarbutton default actions do not work after de-xbl in bug 1547233

Status: RESOLVED FIXED

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

Description

[Magnus Melin [:mkmelin]]

Accept/tentative/decline toolbarbutton default actions do not work after de-xbl in bug 1547233.

For command= the handling is automatic, but these buttons use oncommand, and then we need to handle stopping the event propagation.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1572379_accept_toolbarbutton.patch

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 9083944 [details] [diff] [review]
bug1572379_accept_toolbarbutton.patch

Looks like this is broken in TB 69 beta, so please stick the approval on if you want to fix it there.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 9083944 [details] [diff] [review]
bug1572379_accept_toolbarbutton.patch

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

I see also a few updates to the functions managing attendee status, is this related to the XBL changes as well? Would it be possible to add some tests around that? I seem to recall some of that code has been there for a while so I want to make sure we are not breaking things, e.g. an attendee with an empty participation status value or whatnot.

r+ with those changes.

::: calendar/base/content/dialogs/calendar-summary-dialog.xul
@@ +109,5 @@
>          <menupopup id="declineDropdown">
>            <menuitem id="declineButton_Send"
>                      tooltiptext="&summary.dialog.send.tooltiptext;"
>                      label="&summary.dialog.send.label;"
> +                    oncommand="updatePartStat('DECLINED');saveAndClose('AUTO');event.stopPropagation();"/>

Can you add a space after the semicolon here and in other places?

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #3)

I see also a few updates to the functions managing attendee status, is this
related to the XBL changes as well?

My initial thought was to pass in the event to the (existing) reply() function and stop propagation there. Worked, but it was rather confusing with the three parameters, and then looking closer I notice it's just passing on the one parameter. So to keep the functions about what they do and not have a function do three different tasks at once, I decided to split reply up so that each function would just do one thing.

The dropping of defaults was just to make code more explicit about which case it was doing. There weren't many callers not specifying a parameter anyway.

Would it be possible to add some tests
around that?

While tests for this functionality could be useful, I'm not aware we have anything that tests this, and the particular case I'm fixing would be very hard for an automated test to check (e.g. that something was sent, or not, and only once).

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1572379_accept_toolbarbutton.patch

Eslint didn't like more the previous solution (more than two statements on a line), so I went back to just reply() instead.

Successful try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=45ed859a400ea66cf0c4a2ffcddbedb8180a9da4

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dd2b0261882e
adjust invitation related toolbarbuttons to make default actions work again. r=Fallen

Comment 7

[Jorg K (CEST = GMT+2)]

TB 69 beta 3 / Cal 7.1:
https://hg.mozilla.org/releases/comm-beta/rev/c850972a72351538ece8d91ed11c2f810769df3f