Closed Bug 428538 Opened 16 years ago Closed 15 years ago

Startup notice for calendars of uninstalled/removed providers

Categories

(Calendar :: Internal Components, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

References

Details

Attachments

(3 files)

When uninstalling a provider (i.e gdata), subscribed calendars remain. This currently leads to an error message dialog on each startup for each calendar.

We should think about what to do with this. I would suggest the following:

* Add an attribute containing the provider extension uuid to calICalendarProvider
* observe em-request-action to find out when an extension is being uninstalled.
* if the uuid matches one of the calendar's provider uuids, show a dialog to let
  the user decide which calendars to unsubscribe to.
* Don't signal the error using a dialog at startup. If at all, then via error console.


Alternatively, we could:

* Just remove the calendars from the manager if there is no interface
  => Bad, in case the extension should just be reinstalled
* Just leave in all calendars and ignore the errors
  => Bad, fills up the profile with non-working calendars
  => Good, users get to keep their calendars on reinstall
  => Good, less needed user interaction

Christian, others, whats your opinion on this?
Whiteboard: [qa discussion needed]
I think that calendars should not be automatically removed from the list.  If you decide to do this then I think that users should be asked for permission.

Ultimately what I'd like to see in the calendar list are visual indicators for (1) which calendars are currently loading and (2) which calendars could not be loaded (e.g. because provider extension uninstalled, network down, etc.).  Maybe there could also be a tooltip for each calendar that gives more details.
(In reply to comment #1)
> Ultimately what I'd like to see in the calendar list are visual indicators for
> (1) which calendars are currently loading and (2) which calendars could not be
> loaded (e.g. because provider extension uninstalled, network down, etc.). 
> Maybe there could also be a tooltip for each calendar that gives more details.
+1, this basically fits our "improved error notifications" story. IMO it makes sense to roughly categorize errors, e.g. "user" (e.g. writing a read-only calendar) and "severe" ones (e.g. no network, no provider installed). Each of those needs different UI, and there are possibly more.
Removing [qa discussion needed] from whiteboard. We need a warning mechanism for this case. If we don't want a dialog, we should go for a visualization in the calendar list.
Whiteboard: [qa discussion needed]
Assignee: philipp → nobody
Status: ASSIGNED → NEW
I totally agree with Pete's suggestion without deleting calendars.

Just indicate by striking them in the list and maybe add an explanation into tooltip or on double-click.

It would be great to have a relationship between calendars and its providers, but it would blow the code for fixing this and would not allow to install another provider with the same functionality or to change the uuid of an existing provider in future...
Flags: blocking-calendar1.0?
There's a similar bug 439620 to cope with deployment problems, adding dependency.
Depends on: 439620
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Re-reading this bug, I think we should show the calendars as disabled in the calendar list. The properties dialog should look a bit different.

This is a mockup for the calendar properties dialog as it could look like when the provider was not found. Possibly we can still show more calendar properties depending on implementation.

Unfortunately we probably can't show the extension name that is missing, unless we save that somewhere in the database when the extension is installed. Not sure how feasible that is.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #350501 - Flags: ui-review?(christian.jansen)
This dialog could be shown when the user requests to uninstall the provider.
Attachment #350506 - Flags: ui-review?(christian.jansen)
Attachment #350501 - Attachment is patch: false
Attachment #350501 - Attachment mime type: text/plain → image/png
Comment on attachment 350501 [details]
Screenshot - Calendar Properties Dialog

In general I like the idea, but i think a typical warning with the information of the Calendar name, the unsubscribe option and the Location information would work better in this case.

Please change the dialog to a warning. ui+ with the change.
Comment on attachment 350506 [details]
Screenshot - Uninstall Dialog

It is not really clear to me for what the "Keep Addon" option is good for? should it not be a simple "Cancel", or "Do not remove" In addition I suggest to use "remove" instead of "uninstall".
(In reply to comment #8)
> (From update of attachment 350501 [details])
> In general I like the idea, but i think a typical warning with the information
> of the Calendar name, the unsubscribe option and the Location information would
> work better in this case.
> 
> Please change the dialog to a warning. ui+ with the change.

Christian, this is not a dialog that should be shown at startup, but rather the way the properties dialog should look like when the provider is uninstalled. The way I see it, calendars should look just like they did before  (but disabled) in the calendar list and no startup notification should happen.

(In reply to comment #9)
> (From update of attachment 350506 [details])
> It is not really clear to me for what the "Keep Addon" option is good for?
> should it not be a simple "Cancel", or "Do not remove" In addition I suggest to
> use "remove" instead of "uninstall".

I'm happy to change the strings, I wasn't quite satisfied anyway. "Keep Addon" would mean that the addon is not disabled or uninstalled (Remember this dialog happens when the user clicks "disable" or "uninstall". I'll move to Cancel/Uninstall and Cancel/Disable if you like.
Christian, please respond to comment #10 and reassign back to me.
Assignee: philipp → christian.jansen
Attached patch Fix - v1Splinter Review
This patch requires the code from bug 475715 and adds the UI in a similar fashion. I decided to assume that force-disabled means that the provider is missing, since this is the main reason and the user will get those messages and I don't think its a good idea to confuse the user with all sorts of other scenarios that we don't even know about yet.
Assignee: christian.jansen → philipp
Attachment #359266 - Flags: review?(Berend.Cornelius)
Depends on: 475715
Whiteboard: [needs review, dependant bug]
Whiteboard: [needs review, dependant bug] → [needs review, dependant bug][has l10n impact][needed beta]
Whiteboard: [needs review, dependant bug][has l10n impact][needed beta] → [has l10n impact][needed beta][needs review, dependant bug]
Whiteboard: [has l10n impact][needed beta][needs review, dependant bug] → [needed beta][has l10n impact][needs review, dependant bug]
Attachment #359266 - Flags: review?(berend.cornelius09) → review+
Comment on attachment 359266 [details] [diff] [review]
Fix - v1

I have nothing to complain. Looks and workks very good!
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/90197594f3b6>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needed beta][has l10n impact][needs review, dependant bug]
Target Milestone: --- → 1.0
Correction: rev 90197594f3b6
Comment on attachment 350501 [details]
Screenshot - Calendar Properties Dialog

Patch has already landed. Removing review request.
Attachment #350501 - Flags: ui-review?(chris.j.bugzilla)
Attachment #350506 - Flags: ui-review?(chris.j.bugzilla)
Comment on attachment 350506 [details]
Screenshot - Uninstall Dialog

Patch has already landed. Removing review request.
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.