Implement correct export/print stages and dialog behavior

[sssoleileraaa]

Description

• closes Add dialog titles for export dialogs #642
• Towards Finalize Export messaging and behavior #549

A lot of messaging has been done as a group during meetings and in Slack. We still need to finalize the messaging so this can still be considered a rough draft but closer to what we want.

• Resolves Don't close Export Dialog when the user clicks outside of it #557

In order to do this in Qubes I had to set the window flag to Qt.Widget (some window flags in Qubes do not work unless you tell Qt to bypass the window manager, which is a dangerous thing to do). Since the dialog is a widget, the user can continue to interact with the client while the dialog is open. In order to avoid a race condition, I made it so only one print or export dialog can be opened at a time. Since the dialog cannot be moved, it is very unlikely that the user will try to interact with the client while it is open, but we need to make sure nothing bad happens if they do during test (see Test Plan).

This change also required adding close and cancel buttons to close the dialog since it can no longer be closed by clicking outside of it. Since the cancel button was added, I added a continue button next to it so we could see what the buttons look like together.

• Resolves Add "Speedbump" screen at beginning of Export flow #618

This should have been a separate PR and can be factored out, but will take a bit of effort, so I want to offer a code walk-through first in case that helps. The speedbump was added for both print and export, which was one more reason to create a FramelessDialog class that they could both derive from. The FramelessDialog class sets the window flags and creates the layout and styles that both print and export share. The layout looks like this:

• Resolves updating the UI, see https://app.zeplin.io/project/5c807ea562f734bd2756b243/screen/5ddc3b8b9d9aab94be0542d5 for reference, which has gone through many iterations

• Added filename to PrintDialog so that "Preparing to print" -> "Preparing to print: filename.pdf"

• Added "Export successful" message since we removed it from the Export VM. I created a followup issue to make the color green and icon image to a checkmark.

• Added show/hide password to passphrase form (using the same eye icon as we use in the login dialog)

Test Plan

1. Click outside of dialog for both print and export to verify it no longer closes the dialog
2. Make sure you can only open one dialog at a time, either a single print dialog or a single export dialog
3. Resize the window and see that the dialog continues to be centered with the application
4. Click on "Print" and see that filename now shows up in preparation window
5. Click on "Cancel" to verify that it closes the dialog for both print and export
6. Test show/hide password
7. See "Export successful" message when the export succeeds
8. See dialog close when the Export VM begins opening the 3rd party printer application
9. Make sure you can still export and print documents and test all the error cases: (1) bad passphrase, (2) no usb device connected, (3) multiple devices connected, (4) unencrypted usb device connected

[sssoleileraaa]

Also verify that "Continue" is now the default selection

[sssoleileraaa]

@redshiftzero heads up: a PR for this issue will need to be merged first and then i'll need update this pr to use the new printer pre-flight check so that the client dialog can close when X Printing Panel is in use.

[kushaldas]

Initial review of the code looks good. I will test the changes on Qubes, after @creviera confirms that the other PR is ready/merged.

[eloquence]

This is blocked by completion of freedomofpress/securedrop-export#23 (WIP in a branch).

[sssoleileraaa]

A lot of rebasing since this is getting to be an old PR and updates based on in-person feedback during design reviews. Sorry for the size, I updated the PR description and mentioned that I can split this into two PRs but would be happy to do a code-walkthrough.

BEFORE (first step)

AFTER (first step)

This is ready for review!

[sssoleileraaa]

This is ready for review and should be tested in Qubes. Two known issues: (1) the continue button doesn't change to a slightly light color (reducing opacity) after it's been clicked on yet, and (2) the dialog doesn't recenter when you resize the client window, which i will file followup issues for.

[redshiftzero]

I'm leaving my blocking review here until I check one issue but in the meantime - @kushaldas want to take a spin through the functionality & diff here?

[sssoleileraaa]

I forgot about addressing that comment, thanks @redshiftzero

Instead of splitting the header into two separate widgets (most importantly a QLabel for the filename), we could sanitize the filename input before adding it to the one header widget, as discussed. I'll update this so it'll be ready for kushal's review.

[sssoleileraaa]

I'm leaving my blocking review here until I check one issue but in the meantime - @kushaldas want to take a spin through the functionality & diff here?

just added a commit that addresses this issue (also added a couple tests to ensure filename is sanitized using our standard sensitization method when the dialogs are created)

[kushaldas]

• Click outside of dialog for both print and export to verify it no longer closes the dialog
• Make sure you can only open one dialog at a time, either a single print dialog or a single export dialog
• Resize the window and see that the dialog continues to be centered with the application
• Click on "Print" and see that filename now shows up in preparation window
• Click on "Cancel" to verify that it closes the dialog for both print and export
• Test show/hide password
• See "Export successful" message when the export succeeds
• See dialog close when the Export VM begins opening the 3rd party printer application
• ❌ When I exported a file, it created 3 exported directories in the device.
• ❌ If I connect an un-encrypted USB device, there is no way to know what is going on when I click on "continue" in the export dialog

[sssoleileraaa]

• ❌ When I exported a file, it created 3 exported directories in the device.

It looks like this is a bug in the continue button signal/slot wiring for both print and export operations. I don't know why this is happening, but here's the simplest STR I could come up with:

1. Do not connect a printer
2. run LOGLEVEL=debug python -m securedrop_client
3. click on print button
4. before you click continue button after reading the printer safety tips, tail the logs in both sd-devices for securedrop-export and sd-app for securedrop-client
5. click continue
6. see ERROR_PRINTER_NOT_FOUND log
7. click continue again
8. see multiple ERROR_PRINTER_NOT_FOUND logs

After adding extra log lines while debugging, i see that the problem is coming from the continue button. Either one click is somehow being emitted as multiple clicks or there is something wrong with how I'm connecting the continue button to the slot: self.continue_button.clicked.connect(self._run_preflight).

I'll do some more debugging and perhaps we can discuss this issue further tomorrow. (Nice find)

[sssoleileraaa]

• ❌ If I connect an un-encrypted USB device, there is no way to know what is going on when I click on "continue" in the export dialog

No device

This is what I'm seeing is when no drive is attached (the screen doesn't change if you click continue without attaching a device):

When you click the continue button we don't show an error message in red, which we should, but I'd like to point out that we don't on master either. If there's time tomorrow, I could squeeze this into this PR or we could do it as a followup. Here's what you'll see on master with nothing changing when you click continue with no device attached:

Non-luks encrypted device

This is what I'm seeing is when you click continue and the device attached is not luks encrypted:

Which is an improvement to what we do on master:

[sssoleileraaa]

It looks like this is a bug in the continue button signal/slot wiring for both print and export operations.

Found the bug: rewiring a button signal requires calling disconnect on that signal in order to break previous connections

I fixed this issue and it can be tested both by running through my STR to test print here: #666 (comment) and by running through this procedure to test export:

1. go through the export flow until you successfully transfer a document to a luks-encrypted drive
2. verify only one copy of the document is on your thumb drive

[redshiftzero]

Re-reviewed diff, my comments were addressed except one nit. Testing is looking great, one issue discovered:

✅1. Click outside of dialog for both print and export to verify it no longer closes the dialog
❌2. Make sure you can only open one dialog at a time, either a single print dialog or a single export dialog - I can open an export dialog for doc A, then click on print for another document doc B, and a print dialog for doc B appears on top of the export dialog for doc A.
✅3. Resize the window and see that the dialog continues to be centered with the application - true when I click continue
✅4. Click on "Print" and see that filename now shows up in preparation window
✅5. Click on "Cancel" to verify that it closes the dialog for both print and export
✅6. Test show/hide password
✅7. See "Export successful" message when the export succeeds
✅8. See dialog close when the Export VM begins opening the 3rd party printer application
✅9. Make sure you can still export and print documents and test all the error cases: (1) bad passphrase, (2) no usb device connected, (3) multiple devices connected, (4) unencrypted usb device connected - tested no printer connected case, tested printer connected & success case, printer error case, tested USB not connected case, tested USB connected but not encrypted case, tested USB export success case, tested USB bad passphrase case. All works as expected. 🎉

Dismissing due to reviewer not available for re-review

[redshiftzero]

thanks for the latest changes and your patience @creviera (and your review @kushaldas), your latest changes address my comments, approving!