Feature #15664

Provide feedback during unlocking in GtkPlacesSidebar

Added by sajolida 2018-06-18 15:06:07 . Updated 2018-11-07 10:30:23 .

Status:
Resolved
Priority:
High
Assignee:
Category:
Target version:
Start date:
2018-06-18
Due date:
% Done:

100%

Feature Branch:
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:
299

Description

During the user testing of the VeraCrypt beta, 4 participants out of 5 were confused by the lack of feedback while unlocking a VeraCrypt volume.

  • The lack of feedback is slightly confusing when the unlocking parameters are correct because the operation is shorted and the outcome is happy.
  • The lack of feedback is clearly problematic when the unocking parameters are incorrect because the operation is much longer. People don’t understand what’s going on, they start doing other things, and sometimes even fail to relate the error message to the VeraCrypt operation as it comes so long afterwards.

Merge requests:
https://gitlab.gnome.org/GNOME/gtk/merge_requests/260 (merged)
https://gitlab.gnome.org/GNOME/gtk/merge_requests/261 (merged)


Files


Subtasks


Related issues

Related to Tails - Feature #15663: Improve error messages when unlocking a VeraCrypt volume Resolved 2018-06-18

History

#1 Updated by sajolida 2018-06-18 15:06:26

  • related to Feature #15663: Improve error messages when unlocking a VeraCrypt volume added

#2 Updated by segfault 2018-06-19 18:49:39

  • Status changed from Confirmed to In Progress
  • Assignee changed from segfault to sajolida
  • QA Check set to Info Needed

I successfully added a spinner to the GNOME Shell unlock dialog and backported it to our version of gnome-shell.

But after reading the description again, and with the confusion we had with Feature #15663 in mind, I’m wondering whether that actually is what this ticket is about.

> People don’t understand what’s going on, they start doing other things […]

This part makes me wonder whether you wanted me to add feedback to Disks instead of Shell, because with the Shell dialog, people can’t really start doing other things, because it freezes the desktop. On the other hand Disks, already has feedback in form of a spinner.

#3 Updated by sajolida 2018-06-22 17:13:59

The weird thing is that I don’t understand what you mean by the “GNOME Shell unlock dialog”. For me we were working with 2 dialogs, and 2 different workflows:

  • GVfs from Files
  • Disks, only when using keyfiles

So unless I specifically refer to Disks I’m talking about the workflow outside of Disks.

I’m attaching a screencast to clarify the problem: no feedback with veracrypt.webm.

I want some feedback that makes it clear that some action is being performed in the background during the ~20 seconds between the user clicking “Unlock” and the error message appearing. It could be the pointer turning into a spinner or a spinner in the sidebar of Files but I’m not sure what’s possible or easy given the timeline we have.

I also tested what happens with LUKS and there’s no feedback either. So maybe this bug should be reported upstream as well and might deserve being fixed for both VeraCrypt and LUKS.

#4 Updated by sajolida 2018-06-22 17:14:48

#5 Updated by sajolida 2018-06-22 17:30:25

Later on today I actually got a spinner when trying to mount a USB stick that I had pulled out of another laptop without unmounting. I don’t have the time to investigate exactly what happened and why but I got a spinner when mounting a USB stick so maybe it’s not super crazy to trigger one when unlocking encryption. See screencast.

#6 Updated by segfault 2018-06-26 15:43:21

  • Subject changed from Provide feedback while unlocking a VeraCrypt volume to Provide feedback in GNOME Shell unlock dialog

sajolida wrote:
> The weird thing is that I don’t understand what you mean by the “GNOME Shell unlock dialog”. For me we were working with 2 dialogs, and 2 different workflows:
>
> * GVfs from Files
> * Disks, only when using keyfiles

Ok, thanks for clarifying :) We mean the same dialog then, GVfs doesn’t have a GUI of its own, it triggers the GNOME Shell dialog. (Also, generally, every “modal” dialog, i.e. the ones which force to be in the foreground and make everything else unclickable, and everything else which has the same black style in our theme is GNOME Shell).

> I want some feedback that makes it clear that some action is being performed in the background during the ~20 seconds between the user clicking “Unlock” and the error message appearing. It could be the pointer turning into a spinner or a spinner in the sidebar of Files but I’m not sure what’s possible or easy given the timeline we have.

I agree, there is some feedback missing here too. What I implemented is a spinner in the GNOME Shell dialog, but with your video I realized that the dialog doesn’t stay open if triggered via the GtkPlacesSidebar. Note that when opened by the volume monitor, the dialog does stay open until it’s closed by the user or the volume was successfully unlocked. But since we don’t have automount enabled, the volume monitor doesn’t trigger the dialog in Tails, so that doesn’t actually help us :/

I spent a ridiculously long time on analyzing why exactly the dialog is closed when opened via the sidebar and stays open when opened via the volume monitor. I eventually figured out that the reason is that the places sidebar uses GtkMountOperation, while the volume monitor uses GNOME Shell’s ShellMountOperation. This is not configurable, we would have to patch GtkMountOperation to change this behavior, and I think upstream decided to do it this way for UX reasons.

So I think the best solution would be to add a spinner to the volume entry in the GtkPlacesSidebar. I can try to create a patch for this.

> I also tested what happens with LUKS and there’s no feedback either. So maybe this bug should be reported upstream as well and might deserve being fixed for both VeraCrypt and LUKS.

Yes, since the work has to be done in GTK, we should definitely upstream this. And I’m also going to upstream the new spinner in the GNOME Shell dialog along with our other GNOME Shell patches.

> Later on today I actually got a spinner when trying to mount a USB stick that I had pulled out of another laptop without unmounting. I don’t have the time to investigate exactly what happened and why but I got a spinner when mounting a USB stick so maybe it’s not super crazy to trigger one when unlocking encryption. See screencast.

That’s unrelated, it’s part of Nautilus and appears when opening a directory takes longer than some timeout. Since the GtkPlacesSidebar is also used in other applications, I don’t think we should try to reuse this Nautilus specific solution.

#7 Updated by segfault 2018-06-26 15:43:35

  • QA Check deleted (Info Needed)

#8 Updated by segfault 2018-06-27 09:26:52

  • Subject changed from Provide feedback in GNOME Shell unlock dialog to Provide feedback during unlocking in GtkPlacesSidebar

#9 Updated by sajolida 2018-07-09 10:36:12

  • Deliverable for set to 299

Summary from the meeting today:

  • segfault wrote a patch to keep the GNOME Shell dialog open and display a spinner in it. He’ll upstream it after releasing the beta.
  • VeraCrypt Mounter displays a spinner in the application itself.

#10 Updated by intrigeri 2018-07-09 10:59:15

  • Target version set to Tails_3.9

#11 Updated by segfault 2018-07-09 11:08:57

> segfault wrote a patch to keep the GNOME Shell dialog open and display a spinner in it.

No, that’s a misunderstanding. My patch is only to display a spinner in the Shell dialog. I explained in my long post above the cases I identified in which the dialog closes, and in which it stays opened, and that I don’t think upstream would want to change this.

I think the fix for the case in which the dialog closes is to display a spinner in the application itself, which I did in VeraCrypt Mounter, and will prepare a patch for GtkPlacesSidebar.

#12 Updated by segfault 2018-07-22 15:38:45

  • Description updated
  • % Done changed from 0 to 50

I now have a patch for showing a spinner in the places sidebar and created merge requests for it (see description).

#13 Updated by segfault 2018-07-22 17:58:21

  • % Done changed from 50 to 70

The first merge request was merged: https://gitlab.gnome.org/GNOME/gtk/merge_requests/260

#14 Updated by sajolida 2018-07-26 13:19:30

> The first merge request was merged: https://gitlab.gnome.org/GNOME/gtk/merge_requests/260

Wow, soooo cool!

#15 Updated by segfault 2018-07-30 12:07:43

  • Status changed from In Progress to Resolved
  • % Done changed from 70 to 100

The second merge request was merged: https://gitlab.gnome.org/GNOME/gtk/merge_requests/261

#16 Updated by intrigeri 2018-08-14 19:02:24

  • Assignee deleted (segfault)

#17 Updated by segfault 2018-08-26 09:01:34

  • Status changed from Resolved to In Progress
  • % Done changed from 100 to 80

I had to revert this commit in our Debian package, because it caused tests to fail. I’ll try to fix this in time for 3.9.

#18 Updated by segfault 2018-08-26 09:01:43

  • Assignee set to segfault

#19 Updated by segfault 2018-09-04 08:43:33

  • Target version changed from Tails_3.9 to Tails_3.10.1

segfault wrote:
> I had to revert this commit in our Debian package, because it caused tests to fail. I’ll try to fix this in time for 3.9.

I wasn’t able to fix this in time for 3.9. I will try to fix it for 3.10.

#20 Updated by segfault 2018-09-04 08:46:57

This patch is part of GTK+ 3.24, so a next step could be to compare our package with the Debian package for GTK+ 3.24.

#21 Updated by segfault 2018-09-26 18:46:56

I once again spent many hours on this, but couldn’t fix it. I tried disabling the failing tests and successfully built a package, but when I install it in Tails 3.9, I get the same error as in the tests.

I’m currently inclined to just leave the patch out - we have already have a spinner in Unlock VeraCrypt Volumes and GNOME Disks, and I’m well over budget for this project.

#22 Updated by segfault 2018-10-23 22:36:23

  • Status changed from In Progress to Confirmed
  • Target version deleted (Tails_3.10.1)
  • Deliverable for deleted (299)

#23 Updated by intrigeri 2018-10-31 15:16:35

  • Status changed from Confirmed to In Progress
  • Priority changed from Normal to High
  • Target version set to Tails_3.11
  • QA Check set to Info Needed
  • Deliverable for set to 299

If I got it right, this works fine in GTK+ 3.24 (hence setting “Deliverable for” back to its original value) but not once backported to Stretch. If that’s so, fine, let’s call this done! But I’d like to check so can you please tell me which test fails, exactly, so I verify that it passes when building the sid package?

Adjusting metadata so answering this question (which should allow us to close this ticket) is high on your radar: I’d like to reach a conclusion here in the next few days, before we write the final report to the sponsor.

#24 Updated by segfault 2018-11-06 13:23:08

  • Description updated
  • Assignee changed from segfault to intrigeri
  • QA Check deleted (Info Needed)

> If I got it right, this works fine in GTK+ 3.24 (hence setting “Deliverable for” back to its original value) but not once backported to Stretch.

Yes, it works fine in GTK+ 3.24 and GTK+ 4.

> But I’d like to check so can you please tell me which test fails, exactly, so I verify that it passes when building the sid package?

These tests fail: gtk-reftest, object, objects-finalize, templates. The error message is always

Unable to retrieve object 'busy_spinner' from class template for type 'GtkSidebarRow' while building a 'GtkSidebarRow'

#25 Updated by intrigeri 2018-11-07 08:49:42

  • QA Check set to Ready for QA

#26 Updated by intrigeri 2018-11-07 10:30:24

  • Status changed from In Progress to Resolved
  • Assignee deleted (intrigeri)
  • % Done changed from 80 to 100
  • QA Check changed from Ready for QA to Pass

I confirm these 4 tests pass on sid:

$ curl 'https://buildd.debian.org/status/fetch.php?pkg=gtk%2B3.0&arch=amd64&ver=3.24.1-2&stamp=1538330285&raw=0' \
  | grep -E '^PASS: (gtk-reftest|object|objects-finalize|templates)'   
PASS: object
PASS: objects-finalize
PASS: templates
PASS: gtk-reftest

And there’s no instance of busy_spinner is these logs.