1679813 - remove <deck> from calendar-subscriptions-dialog.xhtml

Status: RESOLVED FIXED

(Calendar :: General, task)

Description

[Khushil Mistry [:khushil324]]

Remove <deck> from https://searchfox.org/comm-central/rev/1fa5ebe1384434e904b33bb7de8f0a3d6e8bfdc5/calendar/base/content/dialogs/calendar-subscriptions-dialog.xhtml#64

Three children can be set hidden/not as appropriate instead of setting selectedIndex on the deck.

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679813_de-deck-calendar-event-dialog-recurrence-xhtml-0.patch

I think there is no use of this children element so I have removed it: https://searchfox.org/comm-central/rev/1fa5ebe1384434e904b33bb7de8f0a3d6e8bfdc5/calendar/base/content/dialogs/calendar-subscriptions-dialog.xhtml#65-67
Now there are only two child elements. Any thoughts on this Aleca?

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9190327 [details] [diff] [review]
Bug-1679813_de-deck-calendar-event-dialog-recurrence-xhtml-0.patch

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

Silly question, but how do I enable the `Find Calendar...` menu item?

::: calendar/base/content/dialogs/calendar-subscriptions-dialog.js
@@ +122,1 @@
>          }

Too many conditions and some unnecessary repetitions here. We can simplify with:
if (!operation.isPending) {
  document.getElementById("calendar-subscriptions-status-busy").hidden = true;
  document.getElementById("calendar-subscriptions-status-nomatches").hidden =
    richListBox.getRowCount() > 0;
}

Comment 3

[Khushil Mistry [:khushil324]]

I have used this to open the dialog:

window.openDialog(
    "chrome://calendar/content/calendar-subscriptions-dialog.xhtml",
    "_blank",
    "chrome,titlebar,modal,resizable"
);

Comment 4

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679813_calendar-subscriptions-dialog-xhtml-1.patch

Comment 5

[Magnus Melin [:mkmelin]]

Ah yes - IIRC the last use for this dialog was WCAP calendars. I think we should remove it since it can't in practice be reached from the UI.

Comment 6

[Khushil Mistry [:khushil324]]

Sure, I will update the patch.

Comment 7

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679813_remove-calendar-subscriptions-dialog-xhtml-0.patch

Comment 8

[Alessandro Castellani [:aleca]]

Comment on attachment 9190502 [details] [diff] [review]
Bug-1679813_remove-calendar-subscriptions-dialog-xhtml-0.patch

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

Good clean up, but you missed a little thing here: https://searchfox.org/comm-central/rev/8783b24ad5915927163977ea19a9968659b90d4c/calendar/base/content/calendar-management.js#615

This is the logic that was showing/hiding the menu item in the context menu.
Be sure that we're not interacting with any of those parts anymore.

Comment 9

[Khushil Mistry [:khushil324]]

Yes, sorry, it was in mind but missed it in removing other files. Updating the patch now.

Comment 10

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679813_remove-calendar-subscriptions-dialog-xhtml-1.patch

Comment 11

[Alessandro Castellani [:aleca]]

Comment on attachment 9190637 [details] [diff] [review]
Bug-1679813_remove-calendar-subscriptions-dialog-xhtml-1.patch

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

LGTM, thanks.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3081153fc8c7
remove calendar subscriptions dialog. r=aleca