From: Murray Cumming Date: Mon, 9 Jul 2007 19:29:53 +0000 (+0000) Subject: 2007-07-09 Murray Cumming X-Git-Tag: git_migration_finished~2881 X-Git-Url: http://git.maemo.org/git/?p=modest;a=commitdiff_plain;h=47b04b1e03f3599c7c77b5040baf7ad58aa3802b 2007-07-09 Murray Cumming * src/maemo/modest-maemo-utils.c: (on_hide), (modest_maemo_show_dialog_and_forget): Handle the response, because the window is not even hidden by default. * src/maemo/modest-account-view-window.c: (on_edit_button_clicked): * src/modest-ui-actions.c: (modest_ui_actions_on_accounts): Use modest_maemo_show_dialog_and_forget() instead of gtk_dialog_run() which seems to prevent some modality problems. * src/maemo/modest-main-window.c: (on_sendqueue_error_happened): Ignore user cancellation errors. * src/modest-account-mgr-helpers.h: * src/modest-account-mgr-helpers.c: Added modest_server_account_get_password() and modest_server_account_get_has_password() to avoid direct use of conf enums. * src/maemo/modest-account-settings-dialog.h: * src/maemo/modest-account-settings-dialog.c: (modest_account_settings_dialog_init): Store the notebook so we can use it later. Added modest_account_settings_dialog_switch_to_user_info(). * src/modest-tny-account-store.c: (get_password): When the password in the account settings is wrong (we think), show the relevant page of the account settings dialog, in the mainloop. However, we currently get many of these at once because cancel does not seem to cancel. (forget_password): Do not forget the password from the account settings, because this causes side-effects when tinymail seems to call it at strange time, maybe because of how we are cancelling get_password(). (modest_tny_account_store_alert): Ignore user cancellations. pmo-trunk-r2657 --- diff --git a/ChangeLog2 b/ChangeLog2 index 5990081..7b7c1cb 100644 --- a/ChangeLog2 +++ b/ChangeLog2 @@ -1,5 +1,42 @@ 2007-07-09 Murray Cumming + * src/maemo/modest-maemo-utils.c: (on_hide), + (modest_maemo_show_dialog_and_forget): Handle the response, because + the window is not even hidden by default. + + * src/maemo/modest-account-view-window.c: (on_edit_button_clicked): + * src/modest-ui-actions.c: (modest_ui_actions_on_accounts): + Use modest_maemo_show_dialog_and_forget() instead of gtk_dialog_run() + which seems to prevent some modality problems. + + * src/maemo/modest-main-window.c: (on_sendqueue_error_happened): + Ignore user cancellation errors. + + * src/modest-account-mgr-helpers.h: + * src/modest-account-mgr-helpers.c: + Added modest_server_account_get_password() and + modest_server_account_get_has_password() to avoid direct use of + conf enums. + + * src/maemo/modest-account-settings-dialog.h: + * src/maemo/modest-account-settings-dialog.c: + (modest_account_settings_dialog_init): Store the notebook so we can + use it later. + Added modest_account_settings_dialog_switch_to_user_info(). + + * src/modest-tny-account-store.c: + (get_password): When the password in the account settings is wrong (we think), + show the relevant page of the account settings dialog, in the mainloop. + However, we currently get many of these at once because cancel does not + seem to cancel. + (forget_password): Do not forget the password + from the account settings, because this causes side-effects when + tinymail seems to call it at strange time, maybe because of how we + are cancelling get_password(). + (modest_tny_account_store_alert): Ignore user cancellations. + +2007-07-09 Murray Cumming + * src/maemo/modest-maemo-utils.h: * src/maemo/modest-maemo-utils.c: Added modest_maemo_show_information_note_and_forget() for use instead of diff --git a/src/maemo/modest-account-settings-dialog.c b/src/maemo/modest-account-settings-dialog.c index e5f7c25..7f10380 100644 --- a/src/maemo/modest-account-settings-dialog.c +++ b/src/maemo/modest-account-settings-dialog.c @@ -980,7 +980,7 @@ modest_account_settings_dialog_init (ModestAccountSettingsDialog *self) { /* Create the notebook to be used by the GtkDialog base class: * Each page of the notebook will be a page of the wizard: */ - GtkNotebook *notebook = GTK_NOTEBOOK (gtk_notebook_new()); + self->notebook = GTK_NOTEBOOK (gtk_notebook_new()); /* Get the account manager object, * so we can check for existing accounts, @@ -999,19 +999,19 @@ modest_account_settings_dialog_init (ModestAccountSettingsDialog *self) self->page_outgoing = create_page_outgoing (self); /* Add the notebook pages: */ - gtk_notebook_append_page (notebook, self->page_account_details, + gtk_notebook_append_page (self->notebook, self->page_account_details, gtk_label_new (_("mcen_ti_account_settings_account"))); - gtk_notebook_append_page (notebook, self->page_user_details, + gtk_notebook_append_page (self->notebook, self->page_user_details, gtk_label_new (_("mcen_ti_account_settings_userinfo"))); - gtk_notebook_append_page (notebook, self->page_incoming, + gtk_notebook_append_page (self->notebook, self->page_incoming, gtk_label_new (_("mcen_ti_advsetup_retrieval"))); - gtk_notebook_append_page (notebook, self->page_outgoing, + gtk_notebook_append_page (self->notebook, self->page_outgoing, gtk_label_new (_("mcen_ti_advsetup_sending"))); GtkDialog *dialog = GTK_DIALOG (self); - gtk_container_add (GTK_CONTAINER (dialog->vbox), GTK_WIDGET (notebook)); + gtk_container_add (GTK_CONTAINER (dialog->vbox), GTK_WIDGET (self->notebook)); gtk_container_set_border_width (GTK_CONTAINER (dialog->vbox), MODEST_MARGIN_HALF); - gtk_widget_show (GTK_WIDGET (notebook)); + gtk_widget_show (GTK_WIDGET (self->notebook)); /* Add the buttons: */ gtk_dialog_add_button (GTK_DIALOG(self), GTK_STOCK_OK, GTK_RESPONSE_OK); @@ -1251,6 +1251,24 @@ void modest_account_settings_dialog_set_account_name (ModestAccountSettingsDialo dialog->modified = FALSE; } +/** Show the User Info tab. + */ +void modest_account_settings_dialog_switch_to_user_info (ModestAccountSettingsDialog *dialog) +{ + const gint page_num = gtk_notebook_page_num (dialog->notebook, dialog->page_user_details); + if (page_num == -1) { + g_warning ("%s: notebook page not found.\n", __FUNCTION__); + } + + /* Ensure that the widget is visible so that gtk_notebook_set_current_page() works: */ + /* TODO: even this hack (recommened by the GTK+ documentation) doesn't seem to work. */ + + gtk_widget_show (dialog->page_user_details); + gtk_widget_show (GTK_WIDGET (dialog->notebook)); + gtk_widget_show (GTK_WIDGET (dialog)); + gtk_notebook_set_current_page (dialog->notebook, page_num); +} + static gboolean save_configuration (ModestAccountSettingsDialog *dialog) { diff --git a/src/maemo/modest-account-settings-dialog.h b/src/maemo/modest-account-settings-dialog.h index 01e7d62..419d26f 100644 --- a/src/maemo/modest-account-settings-dialog.h +++ b/src/maemo/modest-account-settings-dialog.h @@ -49,6 +49,8 @@ typedef struct { ModestAuthProtocol protocol_authentication_incoming; + GtkNotebook *notebook; + GtkWidget *page_account_details; GtkWidget *entry_account_title; GtkWidget *combo_retrieve; @@ -100,6 +102,8 @@ ModestAccountSettingsDialog* modest_account_settings_dialog_new (void); void modest_account_settings_dialog_set_account_name (ModestAccountSettingsDialog *dialog, const gchar* account_name); +void modest_account_settings_dialog_switch_to_user_info (ModestAccountSettingsDialog *dialog); + G_END_DECLS #endif /* _MODEST_ACCOUNT_SETTINGS_DIALOG */ diff --git a/src/maemo/modest-account-view-window.c b/src/maemo/modest-account-view-window.c index 6828ac4..6f08eec 100644 --- a/src/maemo/modest-account-view-window.c +++ b/src/maemo/modest-account-view-window.c @@ -39,6 +39,7 @@ #include "modest-tny-platform-factory.h" #include "maemo/easysetup/modest-easysetup-wizard.h" #include "maemo/modest-account-settings-dialog.h" +#include #include "widgets/modest-ui-constants.h" /* 'private'/'protected' functions */ @@ -234,9 +235,7 @@ on_edit_button_clicked (GtkWidget *button, ModestAccountViewWindow *self) ModestAccountSettingsDialog *dialog = modest_account_settings_dialog_new (); modest_account_settings_dialog_set_account_name (dialog, account_name); - gtk_window_set_transient_for (GTK_WINDOW (dialog), GTK_WINDOW (self)); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_widget_destroy (GTK_WIDGET (dialog)); + modest_maemo_show_dialog_and_forget (GTK_WINDOW (self), GTK_DIALOG (dialog)); g_free (account_name); } diff --git a/src/maemo/modest-maemo-utils.c b/src/maemo/modest-maemo-utils.c index 518fbd1..be36a17 100644 --- a/src/maemo/modest-maemo-utils.c +++ b/src/maemo/modest-maemo-utils.c @@ -525,7 +525,23 @@ modest_maemo_show_information_note_and_forget (GtkWindow *parent_window, const g gtk_widget_show (GTK_WIDGET (dialog)); } +#if 0 +static void +on_hide (GtkDialog *dialog, gpointer user_data) +{ + /* Just destroy the dialog: */ + gtk_widget_destroy (GTK_WIDGET (dialog)); +} +#endif +void modest_maemo_show_dialog_and_forget (GtkWindow *parent_window, GtkDialog *dialog) +{ + gtk_window_set_transient_for (GTK_WINDOW (dialog), parent_window); + + /* Destroy the dialog when it is closed: */ + g_signal_connect (G_OBJECT (dialog), "response", G_CALLBACK (on_response), NULL); + gtk_widget_show (GTK_WIDGET (dialog)); +} void modest_maemo_set_thumbable_scrollbar (GtkScrolledWindow *win, gboolean thumbable) @@ -535,3 +551,4 @@ modest_maemo_set_thumbable_scrollbar (GtkScrolledWindow *win, gboolean thumbable hildon_helper_set_thumb_scrollbar (win, thumbable); #endif /* MODEST_HAVE_HILDON1_WIDGETS */ } + diff --git a/src/maemo/modest-maemo-utils.h b/src/maemo/modest-maemo-utils.h index 9aade8c..ec99530 100644 --- a/src/maemo/modest-maemo-utils.h +++ b/src/maemo/modest-maemo-utils.h @@ -132,6 +132,16 @@ void modest_maemo_utils_setup_images_filechooser (GtkFileChooser *chooser); */ void modest_maemo_show_information_note_and_forget (GtkWindow *parent_window, const gchar* message); +/** modest_maemo_show_dialog_and_forget: + * @parent_window: The window for which the note should be transient. + * @message: The dialog to show. + * + * Show the dialog and destroy it when it is closed, without + * blocking. Use this when you don't want to use gtk_dialog_run(), which might lead + * to hangs. + */ +void modest_maemo_show_dialog_and_forget (GtkWindow *parent_window, GtkDialog *dialog); + void modest_maemo_set_thumbable_scrollbar (GtkScrolledWindow *win, gboolean thumbable); diff --git a/src/maemo/modest-main-window.c b/src/maemo/modest-main-window.c index 6bf6d06..1dd000b 100644 --- a/src/maemo/modest-main-window.c +++ b/src/maemo/modest-main-window.c @@ -455,6 +455,11 @@ on_sendqueue_error_happened (TnySendQueue *self, TnyHeader *header, TnyMsg *msg, { if (err) { printf ("DEBUG: %s: err->code=%d, err->message=%s\n", __FUNCTION__, err->code, err->message); + + if (err->code == TNY_ACCOUNT_ERROR_TRY_CONNECT_USER_CANCEL) + /* Don't show waste the user's time by showing him a dialog telling him + * that he has just cancelled something: */ + return; } /* Get the account name: */ diff --git a/src/modest-account-mgr-helpers.c b/src/modest-account-mgr-helpers.c index d64df94..eef9b81 100644 --- a/src/modest-account-mgr-helpers.c +++ b/src/modest-account-mgr-helpers.c @@ -278,6 +278,29 @@ modest_server_account_set_password (ModestAccountMgr *self, const gchar* account modest_account_mgr_set_string (self, account_name, MODEST_ACCOUNT_PASSWORD, password, TRUE /* server account */); } + + +gchar* +modest_server_account_get_password (ModestAccountMgr *self, const gchar* account_name) +{ + return modest_account_mgr_get_string (self, account_name, MODEST_ACCOUNT_PASSWORD, + TRUE /* server account */); +} + +gboolean +modest_server_account_get_has_password (ModestAccountMgr *self, const gchar* account_name) +{ + gboolean result = FALSE; + gchar *password = modest_account_mgr_get_string (self, account_name, MODEST_ACCOUNT_PASSWORD, + TRUE /* server account */); + if (password && strlen (password)) { + result = TRUE; + } + + g_free (password); + return result; +} + gchar* modest_server_account_get_hostname (ModestAccountMgr *self, const gchar* account_name) diff --git a/src/modest-account-mgr-helpers.h b/src/modest-account-mgr-helpers.h index 269fe14..d5b020e 100644 --- a/src/modest-account-mgr-helpers.h +++ b/src/modest-account-mgr-helpers.h @@ -316,12 +316,31 @@ modest_server_account_set_username_has_succeeded (ModestAccountMgr *self, const * @account_name: The name of a server account. * @password: The new password. * - * Sets the password this server account. + * Sets the password for this server account. */ void modest_server_account_set_password (ModestAccountMgr *self, const gchar* account_name, const gchar* password); - + +/** + * modest_server_account_get_password: + * @self: a ModestAccountMgr instance + * @account_name: The name of a server account. + * + * Gets the password for this server account from the account settings. + */ +gchar* +modest_server_account_get_password (ModestAccountMgr *self, const gchar* account_name); + +/** + * modest_server_account_get_has_password: + * @self: a ModestAccountMgr instance + * @account_name: The name of a server account. + * + * Gets whether a password has been set for this server account in the account settings. + */ +gboolean +modest_server_account_get_has_password (ModestAccountMgr *self, const gchar* account_name); /** * modest_server_account_modest_server_account_get_hostnameget_username: diff --git a/src/modest-tny-account-store.c b/src/modest-tny-account-store.c index e1b9b97..898f34f 100644 --- a/src/modest-tny-account-store.c +++ b/src/modest-tny-account-store.c @@ -51,6 +51,7 @@ #include #include #include +#include #include "modest-tny-account-store.h" @@ -398,36 +399,71 @@ get_account_store_for_account (TnyAccount *account) "account_store")); } +gboolean on_idle_wrong_password (gpointer user_data) +{ + /* TODO: Remember the window per account, + * so we can just reshow it + * instead of showing multiple ones. + */ + + TnyAccount *account = user_data; + + ModestWindow *main_window = + modest_window_mgr_get_main_window (modest_runtime_get_window_mgr ()); + hildon_banner_show_information ( + GTK_WIDGET(main_window), NULL, _("mcen_ib_username_pw_incorrect")); + + const gchar *modest_account_name = + modest_tny_account_get_parent_modest_account_name_for_server_account (account); + if (!modest_account_name) { + g_warning ("%s: modest_tny_account_get_parent_modest_account_name_for_server_account() failed.\n", + __FUNCTION__); + } + + ModestAccountSettingsDialog *dialog = modest_account_settings_dialog_new (); + modest_account_settings_dialog_set_account_name (dialog, modest_account_name); + modest_maemo_show_dialog_and_forget (GTK_WINDOW (main_window), GTK_DIALOG (dialog)); + + g_object_unref (account); + + return FALSE; /* Dont' call this again. */ +} + /* This callback will be called by Tinymail when it needs the password - * from the user, for instance if the password was not remembered. - * It also calls forget_password() before calling this, + * from the user or the account settings. + * It can also call forget_password() before calling this, * so that we clear wrong passwords out of our account settings. * Note that TnyAccount here will be the server account. */ static gchar* get_password (TnyAccount *account, const gchar * prompt_not_used, gboolean *cancel) { - /* Initialize the output parameter: */ + /* TODO: Settting cancel to FALSE does not actually cancel everything. + * We still get multiple requests afterwards, so we end up showing the + * same dialogs repeatedly. + */ + + /* printf ("%s: prompt (not shown) = %s\n", __FUNCTION__, prompt_not_used); */ g_return_val_if_fail (account, NULL); - const gchar *key; - const TnyAccountStore *account_store; - ModestTnyAccountStore *self; + const TnyAccountStore *account_store = NULL; + ModestTnyAccountStore *self = NULL; ModestTnyAccountStorePrivate *priv; gchar *username = NULL; gchar *pwd = NULL; - gpointer pwd_ptr; - gboolean already_asked; + gpointer pwd_ptr = NULL; + gboolean already_asked = FALSE; + /* Initialize the output parameter: */ if (cancel) *cancel = FALSE; - key = tny_account_get_id (account); + const gchar *server_account_name = tny_account_get_id (account); account_store = TNY_ACCOUNT_STORE(get_account_store_for_account (account)); - if (!key || !account_store) { - g_warning ("BUG: could not retrieve account_store for account %s", - key ? key : ""); + if (!server_account_name || !account_store) { + g_warning ("%s: could not retrieve account_store for account %s", + __FUNCTION__, server_account_name ? server_account_name : ""); if (cancel) *cancel = TRUE; @@ -435,31 +471,50 @@ get_password (TnyAccount *account, const gchar * prompt_not_used, gboolean *canc } self = MODEST_TNY_ACCOUNT_STORE (account_store); - priv = MODEST_TNY_ACCOUNT_STORE_GET_PRIVATE(self); + priv = MODEST_TNY_ACCOUNT_STORE_GET_PRIVATE(self); /* This hash map stores passwords, including passwords that are not stored in gconf. */ - /* is it in the hash? if it's already there, it must be wrong... */ + /* Is it in the hash? if it's already there, it must be wrong... */ pwd_ptr = (gpointer)&pwd; /* pwd_ptr so the compiler does not complained about * type-punned ptrs...*/ already_asked = priv->password_hash && g_hash_table_lookup_extended (priv->password_hash, - key, + server_account_name, NULL, (gpointer*)&pwd_ptr); + + printf ("DEBUG: modest: %s: Already asked = %d\n", __FUNCTION__, already_asked); - /* if the password is not already there, try ModestConf */ + /* If the password is not already there, try ModestConf */ if (!already_asked) { - pwd = modest_account_mgr_get_string (priv->account_mgr, - key, MODEST_ACCOUNT_PASSWORD, TRUE); - g_hash_table_insert (priv->password_hash, g_strdup (key), g_strdup (pwd)); + pwd = modest_server_account_get_password (priv->account_mgr, + server_account_name); + g_hash_table_insert (priv->password_hash, g_strdup (server_account_name), g_strdup (pwd)); } - /* if it was already asked, it must have been wrong, so ask again */ - /* TODO: However, when we supply a wrong password to tinymail, - * it seems to (at least sometimes) call our alert_func() instead of - * asking for the password again. - */ + /* If it was already asked, it must have been wrong, so ask again */ if (already_asked || !pwd || strlen(pwd) == 0) { + /* As per the UI spec, if no password was set in the account settings, + * ask for it now. But if the password is wrong in the account settings, + * then show a banner and the account settings dialog so it can be corrected: + */ + const gboolean settings_have_password = + modest_server_account_get_has_password (priv->account_mgr, server_account_name); + printf ("DEBUG: modest: %s: settings_have_password=%d\n", __FUNCTION__, settings_have_password); + if (settings_have_password) { + + + /* The password must be wrong, so show the account settings dialog so it can be corrected: */ + /* We show it in the main loop, because this function might no tbe in the main loop. */ + g_object_ref (account); /* unrefed in the idle handler. */ + g_idle_add (on_idle_wrong_password, account); + + if (cancel) + *cancel = TRUE; + + return NULL; + } + /* we don't have it yet. Get the password from the user */ const gchar* account_id = tny_account_get_id (account); gboolean remember = FALSE; @@ -477,20 +532,18 @@ get_password (TnyAccount *account, const gchar * prompt_not_used, gboolean *canc if (remember) { printf ("%s: Storing username=%s, password=%s\n", __FUNCTION__, username, pwd); - modest_account_mgr_set_string (priv->account_mgr,key, - MODEST_ACCOUNT_USERNAME, - username, TRUE); - modest_account_mgr_set_string (priv->account_mgr,key, - MODEST_ACCOUNT_PASSWORD, - pwd, TRUE); + modest_server_account_set_username (priv->account_mgr, server_account_name, + username); + modest_server_account_set_password (priv->account_mgr, server_account_name, + pwd); } /* We need to dup the string even knowing that it's already a dup of the contents of an entry, because it if it's wrong, then camel will free it */ - g_hash_table_insert (priv->password_hash, g_strdup (key), g_strdup(pwd)); + g_hash_table_insert (priv->password_hash, g_strdup (server_account_name), g_strdup(pwd)); } else { - g_hash_table_remove (priv->password_hash, key); + g_hash_table_remove (priv->password_hash, server_account_name); g_free (pwd); pwd = NULL; @@ -532,8 +585,10 @@ forget_password (TnyAccount *account) } /* Remove from configuration system */ + /* modest_account_mgr_unset (priv->account_mgr, key, MODEST_ACCOUNT_PASSWORD, TRUE); + */ } @@ -966,18 +1021,21 @@ modest_tny_account_store_alert (TnyAccountStore *self, TnyAlertType type, && (error->domain != TNY_ACCOUNT_STORE_ERROR)) { g_warning("%s: Unexpected error domain: != TNY_ACCOUNT_ERROR: %d, message=%s", __FUNCTION__, error->domain, error->message); + return FALSE; } - /* printf("DEBUG: %s: error->message=%s\n", __FUNCTION__, error->message); */ + printf("DEBUG: %s: GError code: %d, message=%s", + __FUNCTION__, error->code, error->message); /* const gchar *prompt = NULL; */ gchar *prompt = NULL; switch (error->code) { case TNY_ACCOUNT_STORE_ERROR_CANCEL_ALERT: - /* Don't show waste the user's time by showing him a dialog telling the - * user that he has just cancelled something: */ + case TNY_ACCOUNT_ERROR_TRY_CONNECT_USER_CANCEL: + /* Don't show waste the user's time by showing him a dialog telling + * him that he has just cancelled something: */ g_debug ("%s: Handling GError domain=%d, code=%d (cancelled) without showing a dialog, message=%s", __FUNCTION__, error->domain, error->code, error->message); prompt = NULL; diff --git a/src/modest-ui-actions.c b/src/modest-ui-actions.c index b7310f4..c398167 100644 --- a/src/modest-ui-actions.c +++ b/src/modest-ui-actions.c @@ -48,6 +48,7 @@ #ifdef MODEST_PLATFORM_MAEMO #include "maemo/modest-osso-state-saving.h" +#include "maemo/modest-maemo-utils.h" #endif /* MODEST_PLATFORM_MAEMO */ #include "widgets/modest-ui-constants.h" @@ -462,9 +463,7 @@ modest_ui_actions_on_accounts (GtkAction *action, ModestWindow *win) } else { /* Show the list of accounts: */ GtkDialog *account_win = GTK_DIALOG(modest_account_view_window_new ()); - gtk_window_set_transient_for (GTK_WINDOW (account_win), GTK_WINDOW(win)); - gtk_dialog_run (account_win); - gtk_widget_destroy (GTK_WIDGET(account_win)); + modest_maemo_show_dialog_and_forget (GTK_WINDOW (win), account_win); } #else GtkWidget *dialog, *label;