1623152 - Make the default "Home" calendar disabled on first run

Status: RESOLVED FIXED

(Calendar :: General, enhancement, P3)

Description

[Paul Morris [:pmorris]]

During bug 1589005 we decided that we want the default "Home" calendar to be disabled on first run. A patch for that was uploaded to that bug, but the calendar tests need more work, so I've opened this bug for finishing this.

Comment 1

[Jorg K (CEST = GMT+2)]

Can you give a bit of background. So someone starts TB out of the box, opens the calendar tab and can't create an event because the default calendar is disabled? You know that the left sidebar showing the calendars can be collapsed, so then it's hard to tell what's going on.

Comment 2

[Paul Morris [:pmorris]]

So, this is one piece of a larger picture. The eventual goal is for there to be no calendar set up until a user sets one up, which gives them the option of whether they want to use a local or networked calendar. (Some users may not want a local calendar, given the widespread use of networked calendars these days.) So this is an intermediate step that moves us closer to that goal. (TB 78 is not that far away.)

The new account central (bug 1589005) and similar improvements will guide users towards setting up existing calendar accounts, and there will be a message in the calendar tab to alert them that all calendars are disabled (bug 1621130), and also the revamped calendar list (bug 1621135) will make it more obvious how to enable calendars and add new calendars.

The collapsible side bar is worth considering, maybe the "all calendars are disabled" message shouldn't be in that sidebar.

Comment 3

[WaltS48 [:walts48]]

In the Calendar tab the Calendar List can be collapsed in the Calendar Pane so no calendars will be showing by clicking the ">" which is normally pointing down to show the list, or hidden completely by using View > Calendar > Calendar Pane and remove the check mark for Calendar List. This also hides the arrow.

The Calendar Pane can be disabled using these steps.

1. The user also needs to be in the Calendar tab.
2. From the Menu bar or button select View > Calendar > Calendar Pane and remove the check mark for Calendar Pane.

Which I'm sure is a known procedure.

I don't see how either of those could be disabled with a first run of Thunderbird with only a Home calendar that is disabled.

Testing the current 76.0a1 with the Home calendar disabled using Properties, the Calendar menu items are not available from the View menu item. The calendar would have to to be enabled.

First run does mean a new profile. No?
Calendar sidebar does mean Calendar Pane. Right?

Comment 4

[Paul Morris [:pmorris]]

The idea in bug 1621135 is for the "calendar pane" (toggled with the ">") to no longer be toggle-able. Similarly I'd assume it makes sense to not let it be hidden either via the view menu.

The "calendar sidebar" is the whole sidebar that includes the minimonth and the calendar pane/list. You can drag to resize that sidebar and dragging it smaller will lead to it being completely hidden. I think that's what Jorgk is concerned about.

Nothing would be hidden on first run, so the user would have to hide things and then forget that they hid them and/or forget what was hidden for this to be an issue. First run does mean a new profile.

Comment 5

[Paul Morris [:pmorris]]

Attachment: home-cal-disabled-on-first-run-0.patch

After trying enabling the "Home" calendar before all tests, I decided it was better to modify the tests to create a new calendar to use and then delete it when they are done. That's more future-proof. This involves using some existing mozmill-style code, so requesting feedback for that, the overall approach, and anything else.

I thought I had all the tests passing locally, but there's more to do, as it looks like at least browser_import.js is failing in this try run.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1d55f10ef5440984a22534d6e27210dae4a85388

(Windows build failures are because I goofed and started an artifact build before windows builds were done.)

Comment 6

[Geoff Lankow (:darktrojan)]

I got a better Try run eventually. It seems using artifact on Try is now busted in more than one way so I went without.

It certainly looks like browser_import.js is broken and maybe browser_timezones.js, but that could be because of the earlier failure.

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

Comment 7

[Geoff Lankow (:darktrojan)]

Comment on attachment 9134297 [details] [diff] [review]
home-cal-disabled-on-first-run-0.patch

Seems okay. I'm surprised how few tests need some changes. It looks like the CalendarUtils.jsm changes are unused now.

Comment 8

[Paul Morris [:pmorris]]

Attachment: home-cal-disabled-on-first-run-1.patch

Okay, the import test and timezones test should be fixed now. Also removed unused utility functions for enabling/disabling calendars. Added some reject() calls in import code. Just started try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4234e3d90c120a303c7c710846b152a082f1498a

Comment 9

[Paul Morris [:pmorris]]

Attachment: home-cal-disabled-on-first-run-2.patch

Fixes timezone test failures. (In the teardown function if we delete the calendar first then there are no events for observers to try to update when you clear the timezone pref, and then they can't cause errors trying to update things that aren't there because we are in the tear down process.)

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0a2dece97d3334447ab15b72b572dee9511f56d1

Comment 10

[Geoff Lankow (:darktrojan)]

Comment on attachment 9134856 [details] [diff] [review]
home-cal-disabled-on-first-run-2.patch

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

::: calendar/base/content/import-export.js
@@ +125,5 @@
>            "_blank",
>            "chrome,titlebar,modal,resizable",
>            args
>          );
> +      } else if (calendars.length == 0) {

I think this is better first, so the checks are 0, 1, and > 1.

::: calendar/test/browser/browser_basicFunctionality.js
@@ +27,5 @@
>  var controller = mozmill.getMail3PaneController();
>  var { eid, lookup } = helpersForController(controller);
>  
>  add_task(function testBasicFunctionality() {
> +  createCalendar(controller, CALENDARNAME);

This test creates a calendar using the UI at the end, just to prove it can. Instead of calling `createCalendar`, let's just move that part to the beginning.

Comment 11

[Paul Morris [:pmorris]]

Attachment: home-cal-disabled-on-first-run-3.patch

Addresses comments from the review. Modified test passes locally.

Comment 12

[Rob Lemley [:rjl]]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fb7372b0b4f7
Let default 'Home' calendar be disabled on first run. r=darktrojan