1476736 - Calendar print preview is empty if a caldav calendar could not be loaded before

Status: RESOLVED FIXED

(Calendar :: Printing, defect)

Description

[[:MakeMyDay]]

STR
1. Have a local and a network calendar without offline support configured
2. Make sure the network calendar it is not available and start TB
3. Switch to calendar view and try to print it

Observed:
The print preview is blank.

Expected:
The print preview should be displayed and should show the events from the local calendar.

This is already an issue with ESR 52, so not related to bug 1101381.

Comment 1

[[:MakeMyDay]]

Attachment: NoPrintingIfCaldavCalendarNotAvailable-V1.diff

This is only a issue if a caldav calendar could not be loaded. All other providers are ok.

Alternatively to passing to this.mOfflineStorage.getItems we could as well notify about an error (what would bring up the unavailable message in the error console when refreshing the print preview).

Comment 2

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

Comment on attachment 8993497 [details] [diff] [review]
NoPrintingIfCaldavCalendarNotAvailable-V1.diff

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

::: calendar/providers/caldav/calDavCalendar.js
@@ +1297,5 @@
>                                               null,
>                                               null);
>              }
> +        } else if (this.checkedServerInfo ||
> +                   !Components.isSuccessCode(this.getProperty("currentStatus"))) {

I think our currentStatus code isn't all that reliable, I am concerned that with this we would get stuck in a place where we'd always getItem from the offline storage. Is there another way to solve this?

Comment 3

[[:MakeMyDay]]

In the use case of this bug the error at that stage is READ_FAILED. So we can limit it to that error and maybe OPERATION_CANCELLED and leaving out any other errors since redaing is what we want to do in getItems.

Comment 4

[[:MakeMyDay]]

Attachment: NoPrintingIfCaldavCalendarNotAvailable-V2.diff

This limits loading the (empty) offline list to the case the calendar is currently in status READ_FAILED, which should be appropriate since this is a read operation where this is in. So using the status here is safe.

This fixes partly also a similar issue of not loading the unifinder list at load time in the same scenario - at least to the extend of being able to get list entries when switching the unifinder filter after the initial load - for the first one the unifinder would need to do lazy loading, but this is beyond the scope if this bug.

Comment 5

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

Comment on attachment 8995102 [details] [diff] [review]
NoPrintingIfCaldavCalendarNotAvailable-V2.diff

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

Very well, we can try this. It does seem we cover most cases of setting currentStatus.

Comment 6

[Jorg K (CEST = GMT+2)]

Could be rephrase the commit message here, it's confusing:
  Allow printing even if at least one caldav calendar is not available.
Do you mean:
  Allow printing even if some caldav calendar is not available.
  Allow printing even if one or more caldav calendars are not available.

Take you pick ;-)

Comment 7

[Jorg K (CEST = GMT+2)]

Oops: Take your pick.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/92d500eaee7f
Allow printing even if some caldav calendar is not available. r=philipp

Comment 9

[[:MakeMyDay]]

Comment on attachment 8995102 [details] [diff] [review]
NoPrintingIfCaldavCalendarNotAvailable-V2.diff

I think this is a candidate for an uplift.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 8995102 [details] [diff] [review]
NoPrintingIfCaldavCalendarNotAvailable-V2.diff

Too late for 63/6.5 beta now.

Comment 11

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

Comment on attachment 8995102 [details] [diff] [review]
NoPrintingIfCaldavCalendarNotAvailable-V2.diff

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

Please keep an eye out for reports once we release the ESR with this patch.

Comment 12

[Jorg K (CEST = GMT+2)]

TB 60.5 ESR, Cal. 6.2.5:
https://hg.mozilla.org/releases/comm-esr60/rev/6ef3a77ffadde78ca6fa11e1cd98a28834fce09a

Target needs to be set to 6.2.5. Philipp?