Re: [evolution-patches] Patch for #59335 - calendar

[Harish Krishnaswamy <harish krishnaswamy gmail com>]

hi,
 
 The current patch addresses the issues (or atleast most of them) that
were causing the bug#59335 in the first place. 
Thanks to chen for his valuable help .. we have also attempted to
explain some of the interesting stuff we were wondering about while
studying this issue..

? why is e_cal_open_async getting called more than once !!!

The create_component_view function in calendar-component.c
invokes the update_selection and update_primary_selection calls in
sequence - that end up calling e_cal_open_async on the same client.
This ensured that e_cal_open_async was called atleast twice everytime.
(when the client was enabled and highlighted at the same time).

> why would cal_opened_cb be called if the calendar has not been opened
> yet?
> 
As a result, the second call of e_cal_open_async encounters a busy
status on the calendar. The open_calendar function currently (that the
patch fixes now) does not set the value of status on any of the exit
points like BUSY and AUTHENTICATION_REQUIRED errors. When these
conditions occur, async_signal_idle_cb sends a cal_opened signal with an
OK status - which explains cal_opened_cb being called before the
calendar has been loaded.

[When the client_cal_opened_cb receives an OK status, it invokes the
e_cal_model_add_client that triggered additional calls to
e_cal_open_async when add_new_client finds the client not to be in
loaded state].

This loop runs for an unspecified number of times before the
authentication is successfully done. 
The listeners on cal_opened_cb in e-cal-model.c were disconnected once
the spurious OK status is received.
Hence, though the calendar was successfully loaded, the views were not
getting updated and required a 'refresh' for the events to show up.

When we tested the patch out, we noticed a few other problems as all the
callback functions bundle 'busy' errors into 'not ok' errors and attempt
to remove the source. The patch handles these scenarios as well..

harish

Index: calendar/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution-data-server/calendar/ChangeLog,v
retrieving revision 1.301
diff -u -p -r1.301 ChangeLog
--- calendar/ChangeLog  28 Jul 2004 12:00:42 -0000      1.301
+++ calendar/ChangeLog  29 Jul 2004 18:33:25 -0000
@@ -1,3 +1,10 @@
+2004-07-29  Harish Krishnaswamy  <kharish novell com>
+       
+       Fixes #59335
+       
+       * libecal/e-cal.c: (open_calendar): Set the status correctly
+       on all exit points from the function.
+
 2004-07-28  Chenthill Palanisamy <pchenthill novell com>
 
        Fixes #60265
Index: calendar/libecal/e-cal.c
===================================================================
RCS file: /cvs/gnome/evolution-data-server/calendar/libecal/e-cal.c,v
retrieving revision 1.68
diff -u -p -r1.68 e-cal.c
--- calendar/libecal/e-cal.c    2 Jun 2004 16:15:00 -0000       1.68
+++ calendar/libecal/e-cal.c    29 Jul 2004 18:33:26 -0000
@@ -1460,6 +1460,7 @@ open_calendar (ECal *ecal, gboolean only
 
        if (ecal->priv->current_op != NULL) {
                g_mutex_unlock (ecal->priv->mutex);
+               *status = E_CALENDAR_STATUS_BUSY;
                E_CALENDAR_CHECK_STATUS (E_CALENDAR_STATUS_BUSY, error);
        }
 
@@ -1477,12 +1478,14 @@ open_calendar (ECal *ecal, gboolean only
 
                if (priv->auth_func == NULL) {
                        g_mutex_unlock (our_op->mutex);
+                       *status =
E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED;
                        E_CALENDAR_CHECK_STATUS
(E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED,
error);
                }
 
                username = e_source_get_property (priv->source,
"username");
                if (!username) {
                        g_mutex_unlock (our_op->mutex);
+                       *status =
E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED;
                        E_CALENDAR_CHECK_STATUS
(E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED,
error);
                }
 
@@ -1494,6 +1497,7 @@ open_calendar (ECal *ecal, gboolean only
                password = priv->auth_func (ecal, prompt, key,
priv->auth_user_data);
                if (!password) {
                        g_mutex_unlock (our_op->mutex);
+                       *status =
E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED; 
                        E_CALENDAR_CHECK_STATUS
(E_CALENDAR_STATUS_AUTHENTICATION_REQUIRED,
error);
                }
 

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/calendar/ChangeLog,v
retrieving revision 1.2464
diff -u -p -r1.2464 ChangeLog
--- ChangeLog   28 Jul 2004 18:03:32 -0000      1.2464
+++ ChangeLog   29 Jul 2004 18:40:24 -0000
@@ -1,3 +1,12 @@
+2004-07-30  Harish Krishnaswamy  <kharish novell com>
+
+       * gui/e-tasks.c: (client_cal_opened_cb),
+       (default_client_cal_opened_cb):
+       * gui/gnome-cal.c: (client_cal_opened_cb),
+       (default_client_cal_opened_cb):
+       Handle E_CALENDAR_STATUS_BUSY conditions and do not 
+       remove the source from the gnome-calendar.
+
 2004-07-27  JP Rosevear  <jpr novell com>
 
        Fixes #62006
Index: gui/gnome-cal.c
===================================================================
RCS file: /cvs/gnome/evolution/calendar/gui/gnome-cal.c,v
retrieving revision 1.347
diff -u -p -r1.347 gnome-cal.c
--- gui/gnome-cal.c     14 Jul 2004 02:20:55 -0000      1.347
+++ gui/gnome-cal.c     29 Jul 2004 18:40:24 -0000
@@ -2013,6 +2013,8 @@ client_cal_opened_cb (ECal *ecal, ECalen
        else
                e_calendar_table_set_status_message (E_CALENDAR_TABLE
(priv->todo),
NULL);
 
+       if (status == E_CALENDAR_STATUS_BUSY)
+               return;
        if (status != E_CALENDAR_STATUS_OK) {
                /* Make sure the source doesn't disappear on us */
                g_object_ref (source);
@@ -2084,6 +2086,9 @@ default_client_cal_opened_cb (ECal *ecal
                e_calendar_view_set_status_message
(priv->views[priv->current_view_type], NULL);
        else
                e_calendar_table_set_status_message (E_CALENDAR_TABLE
(priv->todo),
NULL);
+
+       if (status == E_CALENDAR_STATUS_BUSY)
+               return;
 
        if (status != E_CALENDAR_STATUS_OK) {
                /* Make sure the source doesn't disappear on us */
Index: gui/e-tasks.c
===================================================================
RCS file: /cvs/gnome/evolution/calendar/gui/e-tasks.c,v
retrieving revision 1.100
diff -u -p -r1.100 e-tasks.c
--- gui/e-tasks.c       14 Jul 2004 02:20:55 -0000      1.100
+++ gui/e-tasks.c       29 Jul 2004 18:40:24 -0000
@@ -796,6 +796,8 @@ client_cal_opened_cb (ECal *ecal, ECalen
                set_timezone (tasks);
                set_status_message (tasks, NULL);
                break;
+       case E_CALENDAR_STATUS_BUSY :
+               break;
        default :
                /* Make sure the source doesn't disappear on us */
                g_object_ref (source);
@@ -835,6 +837,8 @@ default_client_cal_opened_cb (ECal *ecal
                
                set_timezone (tasks);
                e_cal_model_set_default_client (model, ecal);
+               break;
+       case E_CALENDAR_STATUS_BUSY:
                break;
        default :
                /* Make sure the source doesn't disappear on us */


-- 
Pure in heart, like uncut jade,
he cleared the muddy water
by leaving it alone.