GNOME Bugzilla – Bug 618019
Notify the user if iDevice needs passcode unlock before being able to connect to it using GVFS
Last modified: 2010-05-30 16:44:41 UTC
During talk on IRC with Bastien he noted about one missing thing in the GVFS afc implementation. If a device has a passcode set on is connected to the computer without having paired anytime in the past, any connection attempt will fail without notice. iFuse already has that functionality and checks for LOCKDOWN_E_PASSWORD_PROTECTED being returned form the lockdownd_client_new_with_handshake() call and informs the user to enter the passcode and retry. We could show some dialog along "Unable to connect to your device. Please unlock it with your password and then press OK" for the user to know about having to enter the passcode and being able to retry the connection with GVFS.
Created attachment 162274 [details] [review] AFC: remove com.apple.afc dependency from afc volume monitor We can get the required information via lockdown even if we are not a trusted host, e.g. when a passcode is set and the device has not been paired before.
Created attachment 162275 [details] [review] AFC: add dialog for passcode protected devices
Comment on attachment 162274 [details] [review] AFC: remove com.apple.afc dependency from afc volume monitor Attachment 162274 [details] pushed as fefbd88 - AFC: remove com.apple.afc dependency from afc volume monitor
Review of attachment 162275 [details] [review]: ::: daemon/gvfsbackendafc.c @@ +270,3 @@ + plist_t value; + lockdownd_error_t lerr; + const gchar *choices[] = {_("Cancel"), _("Try again")}; Should be: const gchar *choices[] = {N_("Cancel"), N_("Try again")}; and then use _(choices[n]) in the actual call. @@ +405,3 @@ + /* translators: + * The first %s is the device name, the second %s is the caption of the + * "Try again" button of the dialog which is defined above. */ Why not simply use "Try again" in the string, and mention that it's the same one as the dialogue button. @@ +406,3 @@ + * The first %s is the device name, the second %s is the caption of the + * "Try again" button of the dialog which is defined above. */ + message = g_strdup_printf(_("Could not connect to device \"%s\" because it has a passcode set. Unlock it by entering the passcode on the device and click \"%s\" to retry."), display_name, choices[1]); Device '%s' has a passcode set. Enter the passcode on the device before clicking 'Try again'. <- better? @@ +414,3 @@ + &aborted, + &choice) + || aborted || (choice == 0)) Could you break that in two lines? ret = g_mount_source_ask_question (...); if (!ret || aborted || choice == 0) break; @@ +467,3 @@ g_free (self->service); g_free(self->model); + g_free(display_name); Could I have a separate patch for the leak fix? It needs to go in the stable branches as well.
(In reply to comment #4) > Review of attachment 162275 [details] [review]: > > ::: daemon/gvfsbackendafc.c > @@ +270,3 @@ > + plist_t value; > + lockdownd_error_t lerr; > + const gchar *choices[] = {_("Cancel"), _("Try again")}; > > Should be: > const gchar *choices[] = {N_("Cancel"), N_("Try again")}; > and then use _(choices[n]) in the actual call. Never mind about that, your usage is correct.
(In reply to comment #4) > Review of attachment 162275 [details] [review]: [...] > @@ +467,3 @@ > g_free (self->service); > g_free(self->model); > + g_free(display_name); > > Could I have a separate patch for the leak fix? It needs to go in the stable > branches as well. This was not meant to be a leak fix, it was inserted as the dialog message should show the device name too. When bailing out unexpectedly, display_name could be non-NULL so I inserted the g_free(). But now I just noticed that it could be freed when it has been freed before, there is missing at least one display_name=NULL statement. We need to review this.
Created attachment 162323 [details] [review] AFC: add dialog for password protected devices
Here's the revised patch. I could not obsolete the previous one.
Added a few spaces before brackets, as we use in GNOME code :) Thanks! Attachment 162323 [details] pushed as 9a8f332 - AFC: add dialog for password protected devices