Re: [evolution-patches] camel provider changes for connector
- From: Sarfraaz Ahmed <asarfraaz novell com>
- To: evolution-patches lists ximian com
- Subject: Re: [evolution-patches] camel provider changes for connector
- Date: Fri, 23 Apr 2004 19:53:19 +0530
Hi,
Thanks a lot for the review comments. Its a pleasure to see my patch getting beautified
As we talked on irc, i have made the changes you mentioned. Attached is the new patch file.
Changes done are as follows :
* Added the response values, and removed the signal calbacks for the dialog buttons
* Made the dialog title and the dialog label on top generic. The providers now need to send the license_name they desire to be displayed on the dialog.
* Modified the gconf entry to be a string type, where each provider's protocol name is added to the list [ once the licesnse agreement is accepted ].
Also created the bug for the HIG modifications. The bug# is 57513.
I am attaching a snapshot of the dialog buttons which now use the stock button for "no" and just a "accpet license" button. I somehow feel that these both do not look good
together .. It would be better if we could also have a stock button for accept license. What are your thoughts on this ?? [ May be i should file a bug for this in glade ?? ]
On Fri, 2004-04-23 at 11:41 +0800, Not Zed wrote:
Looks pretty good, although I was hoping you'd follow my suggestion on irc for the license key. This way there is no need to create any extra key at install time for each license.
e.g.
in the camel provider you have a 'license id', infact if you're going to ONLY have 1 license per provider, you probably don't even need that, you can use the provider name. i.e. 'imap' 'groupwise', etc.
in evolution, you have a single key defined
/apps/evolution/licenses
or even
/apps/evolution/mail/licenses
which is a string list.
it is initially empty.
it contains the id's of all icenses accepted.
The code follows obviously from that.
On Thu, 2004-04-22 at 19:47 +0530, Sarfraaz Ahmed wrote:
Hi Jeff/NotZed,
Thanks a lot for those review comments. I have done the changes you and NotZed had suggested. I have attached the updated patch with this mail. But, i wanted to understand what i was doing wrong, so i have a few silly questions written down in the mail. Please help me understand them better.
I would say get rid of the BUFSIZE macro.
Done
+ GtkTextIter iter;
+ g_sprintf (fname, "%s/../%s", EVOLUTION_GLADEDIR, filename);
this looks pretty dodgy.
1. don't use a static buffer for the filename
2. don't use "../" in a path, better to create a new string macro defined in configure.in or wherever for these license files - or maybe the connector can just hold onto the entire filename itself (and may in fact need to?)
Yeah, that was a lazy workaround ... now, i have modified the code so that the mail provider should directly provide the absolute file name. camel config, just uses it. Apart from that, what's this funda of static buffer ?? I did not understand why i should not be using a static buffer for the filename
+
+ count = fread (filebuf, 1, (BUFSIZE - 1), fd);
count = fread (filebuf, 1, sizeof (filebuf), fd);
Done ...
+ filebuf [count] = '\0';
get rid of this statement
Done ...
+ gtk_text_buffer_insert (buffer, &iter, filebuf, -1);
gtk_text_buffer_insert (buffer, &iter, filebuf, count);
Done ... but wanted to understand why using -1, and the null termination was wrong ..
+ }
+ accepted = gconf_client_get_bool (gconf, prov->gconf_license_bool_key, NULL);
I don't like prov->gconf_license_bool_key. a different name needs to be used and it shouldn't be a gconf key.
maybe something more like prov->license which is a string that the mailer appends to another string, "/apps/evolution/mail/licenses/%s_accepted" to get the actual gconf key.
Yeah, your solution looks pretty neat. Now, i have modified the code so that the mail providers will create their respective gconf keys when they get installed [ so there are no new changes in the evolution camel schema files ], and all the providers would have to create their own keys under /apps/evolution/mail/licenses .. with the key name being <provider_specified_name>_accepted.
+
Thanks
-- Sarfraaz Ahmed <asarfraaz novell com>
|
Thanks
-- Sarfraaz Ahmed <asarfraaz novell com>
|
Index: camel/camel-provider.h
===================================================================
RCS file: /cvs/gnome/evolution/camel/camel-provider.h,v
retrieving revision 1.41
diff -u -r1.41 camel-provider.h
--- camel/camel-provider.h 23 Apr 2004 04:16:04 -0000 1.41
+++ camel/camel-provider.h 23 Apr 2004 14:02:10 -0000
@@ -186,10 +186,9 @@
*/
char *translation_domain;
- /* This string points to the provider's gconf key value
- */
- const char *license;
-
+ /* Name of the provider to be displayed for the license agreement */
+ char *license_name;
+
/* This holds the license file name [ ascii text format ] containing
* the license agreement. This should be the absolute file path. This
* is read only when the HAS_LICENSE flag is set
Index: mail/mail-account-gui.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-account-gui.c,v
retrieving revision 1.159
diff -u -r1.159 mail-account-gui.c
--- mail/mail-account-gui.c 23 Apr 2004 04:15:22 -0000 1.159
+++ mail/mail-account-gui.c 23 Apr 2004 14:02:41 -0000
@@ -157,21 +157,6 @@
g_free (value);
}
-static void
-set_license_rejected (GtkWidget *widget, gpointer data)
-{
- gtk_dialog_response (GTK_DIALOG (data), GTK_RESPONSE_DELETE_EVENT);
- return;
-}
-
-static void
-set_license_accepted (GtkWidget *widget, gpointer data)
-{
- gtk_dialog_response (GTK_DIALOG (data), GTK_RESPONSE_ACCEPT);
- return;
-
-}
-
static void
check_button_state (GtkToggleButton *button, gpointer data)
{
@@ -217,7 +202,7 @@
}
static gboolean
-display_license(const char *filename)
+display_license (const char *filename, const char *lic_name)
{
GladeXML *xml;
GtkWidget *top_widget;
@@ -225,6 +210,8 @@
GtkButton *button_yes, *button_no;
GtkCheckButton *check_button;
GtkResponseType response;
+ GtkLabel *top_label;
+ char *label_text, *dialog_title;
gboolean status;
xml = glade_xml_new (EVOLUTION_GLADEDIR "/mail-license.glade",
@@ -246,25 +233,36 @@
check_button = GTK_CHECK_BUTTON (glade_xml_get_widget (xml,
"lic_checkbutton"));
+ top_label = GTK_LABEL (glade_xml_get_widget (xml, "lic_top_label"));
+
+ label_text = g_strdup_printf ("
+ Please read carefully the license agreement
+ for %s displayed below
+ and tick the check box for accepting it
+ ", lic_name);
+
+ gtk_label_set_label (top_label, label_text);
+
+ dialog_title = g_strdup_printf ("%s License Agreement", lic_name);
+
+ gtk_window_set_title (GTK_WINDOW (top_widget), dialog_title);
+
g_signal_connect (check_button, "toggled",
G_CALLBACK (check_button_state), button_yes);
- g_signal_connect (button_yes, "clicked",
- G_CALLBACK (set_license_accepted),
- GTK_WIDGET (top_widget));
- g_signal_connect (button_no, "clicked",
- G_CALLBACK (set_license_rejected),
- GTK_WIDGET (top_widget));
-
+
response = gtk_dialog_run (GTK_DIALOG (top_widget));
- if (response == GTK_RESPONSE_ACCEPT) {
- gtk_widget_destroy (top_widget);
- g_object_unref (xml);
- return TRUE;
- }
+
+ g_free (label_text);
+ g_free (dialog_title);
+
failed:
gtk_widget_destroy (top_widget);
g_object_unref (xml);
- return FALSE;
+
+ if (response == GTK_RESPONSE_ACCEPT)
+ return TRUE;
+ else
+ return FALSE;
}
static gboolean
@@ -331,32 +329,53 @@
mail_account_gui_check_for_license (CamelProvider *prov)
{
GConfClient *gconf;
- gboolean accepted, status;
- char *gconf_license_key;
+ gboolean accepted = FALSE, status;
+ char * accepted_providers;
+ char **providers_list;
+ char * tmp_providers;
+ char * new_providers;
+ int i;
if (prov->flags & CAMEL_PROVIDER_HAS_LICENSE) {
gconf = mail_config_get_gconf_client ();
- gconf_license_key = g_strdup_printf (
- "/apps/evolution/mail/licenses/%s_accepted",
- prov->license);
- accepted = gconf_client_get_bool (gconf, gconf_license_key,
- NULL);
+ accepted_providers = gconf_client_get_string (gconf,
+ "/apps/evolution/mail/licenses", NULL);
+ providers_list = g_strsplit (accepted_providers, " ", 0);
+ for (i = 0; providers_list && providers_list[i]; i++) {
+ if (!strcmp (providers_list[i], prov->protocol)) {
+ accepted = TRUE;
+ break;
+ }
+ }
if (!accepted) {
/* Since the license is not yet accepted, pop-up a
* dialog to display the license agreement and check
* if the user accepts it
*/
- if (display_license (prov->license_file)) {
- status = gconf_client_set_bool (gconf,
- gconf_license_key, TRUE, NULL);
+ if (display_license (prov->license_file,
+ prov->license_name)) {
+ tmp_providers = g_strdup ("");
+ for (i = 0; providers_list && providers_list[i]; i++) {
+ new_providers = g_strconcat (
+ tmp_providers, " ",
+ providers_list[i], NULL);
+ g_free (tmp_providers);
+ tmp_providers = new_providers;
+ }
+ new_providers = g_strconcat (tmp_providers,
+ " ", prov->protocol, NULL);
+ status = gconf_client_set_string (gconf,
+ "/apps/evolution/mail/licenses",
+ new_providers, NULL);
+ g_free (new_providers);
+ g_free (tmp_providers);
} else {
- g_free (gconf_license_key);
return FALSE;
}
}
- g_free (gconf_license_key);
+ g_strfreev (providers_list);
}
return TRUE;
}
Index: mail/mail-license.glade
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-license.glade,v
retrieving revision 1.1
diff -u -r1.1 mail-license.glade
--- mail/mail-license.glade 23 Apr 2004 06:15:39 -0000 1.1
+++ mail/mail-license.glade 23 Apr 2004 14:07:36 -0000
@@ -5,7 +5,7 @@
<widget class="GtkDialog" id="lic_dialog">
<property name="visible">True</property>
- <property name="title" translatable="yes">Exchange Connector License Agreement for Evolution</property>
+ <property name="title" translatable="yes">License Agreement</property>
<property name="type">GTK_WINDOW_TOPLEVEL</property>
<property name="window_position">GTK_WIN_POS_NONE</property>
<property name="modal">False</property>
@@ -29,10 +29,10 @@
<property name="visible">True</property>
<property name="can_default">True</property>
<property name="can_focus">True</property>
- <property name="label" translatable="yes">NO</property>
- <property name="use_underline">True</property>
+ <property name="label">gtk-no</property>
+ <property name="use_stock">True</property>
<property name="relief">GTK_RELIEF_NORMAL</property>
- <property name="response_id">0</property>
+ <property name="response_id">-6</property>
</widget>
</child>
@@ -41,10 +41,10 @@
<property name="visible">True</property>
<property name="can_default">True</property>
<property name="can_focus">True</property>
- <property name="label" translatable="yes">YES</property>
+ <property name="label" translatable="yes">Accept License</property>
<property name="use_underline">True</property>
<property name="relief">GTK_RELIEF_NORMAL</property>
- <property name="response_id">0</property>
+ <property name="response_id">-3</property>
</widget>
</child>
</widget>
@@ -66,8 +66,7 @@
<widget class="GtkLabel" id="lic_top_label">
<property name="visible">True</property>
<property name="label" translatable="yes">
- Please read carefully the license agreement
- for Evolution Exchange Connector displayed
+ Please read carefully the license agreement displayed
below and tick the check box for accepting it
</property>
<property name="use_underline">False</property>
Attachment:
evo-license-stock.jpg
Description: JPEG image
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
