Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print improvements: don't print unprintable filetypes, improve PDF conversion and mimetype detection #2166

Merged
merged 12 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ jobs:
- name: Install dependencies
run: |
poetry -C ${{ matrix.component }} install
if [[ "${{ matrix.component }}" == "client" ]]; then
if [[ "${{ matrix.component }}" == "client" || "${{ matrix.component }}" == "export" ]]; then
make -C ${{ matrix.component }} ci-install-deps
fi
- name: Run lint
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ jobs:
if: ${{ matrix.component == 'proxy' }}
- name: Install dependencies
run: |
if [[ "${{ matrix.component }}" == "export" ]]; then
make -C ${{ matrix.component }} ci-install-deps
fi
sudo -u user poetry -C ${{ matrix.component }} install
- name: Run test
run: |
Expand Down
10 changes: 5 additions & 5 deletions client/securedrop_client/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def _on_print_prefight_error(self) -> None:
if self.process:
err = self.process.readAllStandardError().data().decode("utf-8").strip()
logger.debug(f"Print preflight error: {err}")
self.print_preflight_check_failed.emit(ExportStatus.ERROR_PRINT)
self.print_preflight_check_failed.emit(ExportStatus.CALLED_PROCESS_ERROR)

def _on_print_complete(self) -> None:
"""
Expand Down Expand Up @@ -303,15 +303,15 @@ def end_process(self) -> None:

def _on_print_error(self) -> None:
"""
Error callback for print qrexec.
Error callback for print qrexec. Called if QProcess fails.
"""
self._cleanup_tmpdir()
if self.process:
err = self.process.readAllStandardError()
logger.debug(f"Print error: {err}")
else:
logger.error("Print error (stderr unavailable)")
self.print_failed.emit(ExportStatus.ERROR_PRINT)
self.print_failed.emit(ExportStatus.CALLED_PROCESS_ERROR)

def print(self, filepaths: list[str]) -> None:
"""
Expand All @@ -332,8 +332,8 @@ def print(self, filepaths: list[str]) -> None:
self._run_qrexec_export(archive_path, self._on_print_complete, self._on_print_error)

except OSError as e:
logger.error("Export failed")
logger.debug(f"Export failed: {e}")
logger.error("Print failed")
logger.debug(f"Print failed: {e}")
self.print_failed.emit(ExportStatus.ERROR_PRINT)

# ExportStatus.ERROR_MISSING_FILES
Expand Down
3 changes: 3 additions & 0 deletions client/securedrop_client/export_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class ExportStatus(Enum):

# Print error
ERROR_PRINT = "ERROR_PRINT"
ERROR_UNPRINTABLE_TYPE = "ERROR_UNPRINTABLE_TYPE"
ERROR_MIMETYPE_UNKNOWN = "ERROR_MIMETYPE_UNKNOWN"
ERROR_MIMETYPE_DISCOVERY = "ERROR_MIMETYPE_DISCOVERY"

# New
PRINT_PREFLIGHT_SUCCESS = "PRINTER_PREFLIGHT_SUCCESS"
Expand Down
63 changes: 51 additions & 12 deletions client/securedrop_client/gui/conversation/export/print_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ def __init__(self, device: Export, file_name: str, filepaths: list[str]) -> None
self.file_name = SecureQLabel(
file_name, wordwrap=False, max_length=self.FILENAME_WIDTH_PX
).text()
# Hold onto the error status we receive from the Export VM
self.error_status: ExportStatus | None = None

# Hold onto the error status we receive from the Export VM. Only required
# because we're reusing the same modal dialog with different text depending
# on conditions, and need to pass methods to Qt handlers with a predefined
# message signature.
self.status: ExportStatus | None = None

# Connect device signals to slots
self._device.print_preflight_check_succeeded.connect(
Expand Down Expand Up @@ -61,6 +65,7 @@ def __init__(self, device: Export, file_name: str, filepaths: list[str]) -> None
)
self.insert_usb_message = _("Please connect your printer to a USB port.")
self.generic_error_message = _("See your administrator for help.")
self.unprintable_type_error_message = _("This file type cannot be printed.")

self._show_starting_instructions()
self.start_animate_header()
Expand All @@ -80,14 +85,38 @@ def _show_insert_usb_message(self) -> None:
self.error_details.hide()
self.adjustSize()

def _show_unprintable_message(self) -> None:
self.continue_button.clicked.disconnect()
self.continue_button.clicked.connect(self.close)
self.header.setText(self.error_header)
self.body.setText(self.unprintable_type_error_message)
self.error_details.hide()
self.adjustSize()

def _show_generic_error_message(self) -> None:
self._show_error_message()

def _show_error_message(self) -> None:
"""
Show error message based on ExportStatus returned.
"""
self.continue_button.clicked.disconnect()
self.continue_button.clicked.connect(self.close)
self.continue_button.setText(_("DONE"))
self.header.setText(self.error_header)
self.body.setText( # nosemgrep: semgrep.untranslated-gui-string
f"{self.error_status}: {self.generic_error_message}"
)
if self.status == ExportStatus.ERROR_UNPRINTABLE_TYPE:
body_text = self.unprintable_type_error_message
else:
body_text = self.generic_error_message

if self.status:
self.body.setText( # nosemgrep: semgrep.untranslated-gui-string
f"{self.status.value}: {body_text}"
)
else:
self.body.setText( # nosemgrep: semgrep.untranslated-gui-string
body_text
)
self.error_details.hide()
self.adjustSize()

Expand All @@ -100,20 +129,30 @@ def _print_file(self) -> None:
self.start_animate_activestate()
self._device.print(self.filepaths)

@pyqtSlot()
def _on_print_complete(self) -> None:
@pyqtSlot(object)
def _on_print_complete(self, status: ExportStatus) -> None:
"""
Send a signal to close the print dialog.
Send a signal to close the print dialog or display
an appropriate error message.
"""
self.status = status
self.stop_animate_activestate()
self.close()
if status == ExportStatus.PRINT_SUCCESS:
self.close()
elif self.status == ExportStatus.ERROR_PRINTER_NOT_FOUND:
self._show_insert_usb_message()
elif self.status == ExportStatus.ERROR_UNPRINTABLE_TYPE:
self._show_unprintable_message()
else:
self._show_error_message()

@pyqtSlot(object)
def _on_print_preflight_check_succeeded(self, status: ExportStatus) -> None:
# We don't use the ExportStatus for now for "success" status,
# but in future work we will migrate towards a wizard-style dialog, where
# success and intermediate status values all use the same PyQt slot.
# If the continue button is disabled then this is the result of a background preflight check
self.status = status
self.stop_animate_header()
self.header_icon.update_image("printer.svg", svg_size=QSize(64, 64))
self.header.setText(self.ready_header)
Expand All @@ -128,20 +167,20 @@ def _on_print_preflight_check_succeeded(self, status: ExportStatus) -> None:

@pyqtSlot(object)
def _on_print_preflight_check_failed(self, status: ExportStatus) -> None:
self.status = status
self.stop_animate_header()
self.header_icon.update_image("printer.svg", svg_size=QSize(64, 64))
self.error_status = status
# If the continue button is disabled then this is the result of a background preflight check
if not self.continue_button.isEnabled():
self.continue_button.clicked.disconnect()
if status == ExportStatus.ERROR_PRINTER_NOT_FOUND:
self.continue_button.clicked.connect(self._show_insert_usb_message)
else:
self.continue_button.clicked.connect(self._show_generic_error_message)
self.continue_button.clicked.connect(self._show_error_message)

self.continue_button.setEnabled(True)
self.continue_button.setFocus()
elif status == ExportStatus.ERROR_PRINTER_NOT_FOUND:
self._show_insert_usb_message()
else:
self._show_generic_error_message()
self._show_error_message()
5 changes: 4 additions & 1 deletion client/securedrop_client/locale/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ msgstr ""
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Generated-By: Babel 2.12.1\n"
"Generated-By: Babel 2.15.0\n"

msgid "{application_name} is already running"
msgstr ""
Expand Down Expand Up @@ -366,6 +366,9 @@ msgstr ""
msgid "See your administrator for help."
msgstr ""

msgid "This file type cannot be printed."
msgstr ""

msgid "YES, DELETE ENTIRE SOURCE ACCOUNT"
msgstr ""

Expand Down
Loading