After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 618019 - Notify the user if iDevice needs passcode unlock before being able to connect to it using GVFS
Notify the user if iDevice needs passcode unlock before being able to connect...
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: afc backend and volume monitor
git master
Other Linux
: Normal enhancement
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-07 14:48 UTC by Martin Szulecki
Modified: 2010-05-30 16:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
AFC: remove com.apple.afc dependency from afc volume monitor (3.23 KB, patch)
2010-05-29 16:20 UTC, Bastien Nocera
committed Details | Review
AFC: add dialog for passcode protected devices (6.69 KB, patch)
2010-05-29 16:21 UTC, Bastien Nocera
needs-work Details | Review
AFC: add dialog for password protected devices (7.08 KB, patch)
2010-05-30 15:37 UTC, Nikias Bassen
committed Details | Review

Description Martin Szulecki 2010-05-07 14:48:39 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.
Comment 1 Bastien Nocera 2010-05-29 16:20:53 UTC
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.
Comment 2 Bastien Nocera 2010-05-29 16:21:42 UTC
Created attachment 162275 [details] [review]
AFC: add dialog for passcode protected devices
Comment 3 Bastien Nocera 2010-05-29 16:28:15 UTC
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
Comment 4 Bastien Nocera 2010-05-29 16:41:08 UTC
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.
Comment 5 Bastien Nocera 2010-05-29 16:45:06 UTC
(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.
Comment 6 Nikias Bassen 2010-05-29 19:51:00 UTC
(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.
Comment 7 Nikias Bassen 2010-05-30 15:37:31 UTC
Created attachment 162323 [details] [review]
AFC: add dialog for password protected devices
Comment 8 Nikias Bassen 2010-05-30 15:38:41 UTC
Here's the revised patch. I could not obsolete the previous one.
Comment 9 Bastien Nocera 2010-05-30 16:44:37 UTC
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