1521417 - Some items of calendar list context menu are permanently disabled

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, enhancement)

Description

[[:MakeMyDay]]

In calendar list context menu, some items (Remove calendar, Publish calendar, properties,...) are permanently disabled, while accessing e.g. the calendar properties dialog is still possible using the main menu.

This is a fallout from bug 1482389, which changed the behaviour of getCellAt to not support out parameters anymore but return values instead.

Comment 1

[[:MakeMyDay]]

Attachment: CalendarPropertiesDialogNotAccesssible-V1.diff

The only other caller of getCellAt in calendar was already converted in bug 1491269.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9f1580781aa3
Use getCellAt return values instead of output parameters; r=philipp

Comment 3

[:aceman]

Comment on attachment 9037863 [details] [diff] [review]
CalendarPropertiesDialogNotAccesssible-V1.diff

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

::: calendar/base/content/widgets/calendar-list-tree.xml
@@ +648,5 @@
>                  aRow = aRow || {};
> +                aCol = aCol || {};
> +                let { col, row } = this.treebox.getCellAt(event.clientX, event.clientY);
> +                aCol.value = col;
> +                aRow.value = row;

Are you sure that this works?
Most other callers use let cell = tree.getCellAt() and then use cell.row and cell.col. See also
https://searchfox.org/comm-central/source/mozilla/dom/chrome-webidl/XULTreeElement.webidl#132 .

Why do you assign to aCol.value and aRow.value if you could use col/row or (cell.col/cell.row) directly? aCol and aRow isn't passed anywhere as an object. I see they are used as out-arguments. The 2 callers could be updated easily to not access them as objects anymore (aCol.value and aRow.value).