285014 - double-clicking on a calendar in the calendars tab does nothing

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

double-clicking on a calendar on the calendars tab should open a dialog to edit
its properties. But it does nothing. 
(a temporary solution would be to open the wizard for it)

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

also, right clicking and selecting edit doesn't work. That should just do the same.

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

Should it be possible to change the type of a calendar? say from storage to caldav?
Or is just editing the parameters like the uri and name enough? (it's easier,
that's for sure)

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

This patch actually doesn't seem to fix doubleclick, i need to investigate
that. It does fix the context menu though.
Also, it actually makes the list live, so that changes are made visible.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I would say that you can change neither URI nor type for an existing calendar. 
Name, highlight colour, whatever other prefs make sense for that type.

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 181617 [details] [diff] [review]
patch v1

>+    content/calendar/calendarManagment.js (content/calendarManagment.js)

Here and elsewhere: you probably want to name it calendarManag_e_ment.js.

> function debug(text)
> {
>-    if(gDebugEnabled)
>-        dump(text + "\n");
>+//    if(gDebugEnabled)
>+//        dump(text + "\n");
> }

Didja figure out what you wanted to do here?  (Fix gDebugEnabled,
just remove those lines, define a new empty debug
function if gDebugEnabled, etc.)

> 
>-   updateColors();
>+   initCalendarManager();
>+
>+//   updateColors();

I'd say to just remove that line, instead of commenting
it out, but that's somewhat just personal preference.

>+<!--
>               <listhead>
>-                <listheader flex="1" crop="end" label="&calendar.calendarlistbox.label;"/>
>                 <listheader/>
>+                <listheader/>
>+                <listheader flex="1" crop="end" label="&calendar.calendarlistbox.label;"/>
>               </listhead>
>+-->

Temporary commenting, or more deletion-candidate?

>+function getDisplayComposite()
>+{
>+    if (!gDisplayComposite) {
>+       gDisplayComposite = Components.classes["@mozilla.org/calendar/calendar;1?type=composite"]
>+                                     .createInstance(Components.interfaces.calICompositeCalendar);
>+       var calMgr = getCalendarManager();
>+       var calList = calMgr.getCalendars({});
>+       for (i in calList) {
>+           gDisplayComposite.addCalendar(calList[i]);
>+       }
>+    }
>+    return gDisplayComposite;
> }

Mmmm, is the right thing for the composite to really return all
calendars known to the manager?  If this is intended to be shared
by Lightning, I don't think it is, but I don't think that's what
Sunbird wants either; how do we honour the checking of items in
the list, etc.?

>@@ -170,104 +162,20 @@ function CalendarWindow( )
>                 calendarWindow.currentView.refreshEvents();
>         },
>         onDeleteItem: function(aDeletedItem) {
>             if (!this.mInBatch)
>                 calendarWindow.currentView.refreshEvents();
>         },
>         onAlarm: function(aAlarmItem) {},
>         onError: function(aMessage) {}
>     }
>+    var ccalendar = getDisplayComposite();
>+    ccalendar.addObserver(calendarObserver, ccalendar.ITEM_FILTER_TYPE_ALL);

Since some recent commit of Vlad's, we don't take a filter parameter on
addObserver.  (Not quite sure why, other than that it was likely not correctly
implemented anywhere.)

Fix the calendarManagement.js naming and the addObserver call, and this
can go in.  You probably want to tidy the comment/delete stuff too,
I guess.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #5)
> Mmmm, is the right thing for the composite to really return all
> calendars known to the manager?

No, it isn't the right thing to do, but it works for now. I need to to think of
a place where to store the current list of selected calendars, without having to
do the housekeeping twice.

I fixed the comments and naming, and checked in. Leaving this bug open for now
to fix the actual issue from the summary :)

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: update views and trees (checked in)

I found out that in sb0.2 double clicking also does nothing, so i guess the
original summary was wrong.
Anyway, this patch makes the event and todo lists and the views observe changes
to the calendar list, so that they will be redrawn when the users (de)selects a
calendar.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: open wizard when no calendars are defined

additional patch, to open the wizard when no calendar is defined, like on first
startup

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 182061 [details] [diff] [review]
update views and trees (checked in)

r=shaver

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 182491 [details] [diff] [review]
open wizard when no calendars are defined

this patch i no longer needed, now bug 296194 is fixed.

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

I don't think there is anything left to be done here. If there is, we can open
new bugs.

Comment 12

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!