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

Dangerzone is listed as a PDF viewer to open safe documents with on Linux #790

Closed
naglis opened this issue Apr 30, 2024 · 4 comments · Fixed by #793
Closed

Dangerzone is listed as a PDF viewer to open safe documents with on Linux #790

naglis opened this issue Apr 30, 2024 · 4 comments · Fixed by #793
Milestone

Comments

@naglis
Copy link
Contributor

naglis commented Apr 30, 2024

Tested on Arch Linux/Ubuntu 22.04 when Dangerzone 0.6.0 is installed as a package. This only applies to Linux systems.

Screenshot of Dangerzone 0.6.0 running on Ubuntu 22.04 showing Dangerzone listed as an application to open safe PDF files after conversion

On Linux, Dangerzone allows to select a PDF viewer to use to open safe PDF documents after conversion. The list of available PDF viewers is populated by parsing the .desktop entries of all available applications and returning ones that can open files with application/pdf mimetype. Currently, Dangerzone itself is included in this list (if Dangerzone is installed as a package system-wide), although in the code there is a specific check to exclude Dangerzone from the list, however the check is done on the lowercase name dangerzone and the application name from the .desktop entry (which is Dangerzone) and so the condition never matches.

I believe it makes sense to exclude Dangerzone from the list of PDF viewers as Dangerzone cannot display PDFs (at least, not currently, see also #424) and repeatedly converting a safe document does not make much sense.

The trivial fix would be to use Dangerzone as the application name in the check, however, IIUC pyxdg DesktopEntry.getName() returns a localized name and if Dangerzone is translated in the future (#132), the Name key in the Dangerzone's .desktop file might differ in other languages. To avoid that, DesktopEntry.get('Name', locale=False) could be used to ignore localizations. Another option could be to use a different way to identify the Dangerzone application, e.g. the name of the .desktop file (press.freedom.dangerzone).

@apyrgio
Copy link
Contributor

apyrgio commented Apr 30, 2024

That's a very nice dig! I'd go with the trivial fix for now and not future-proofing the application, since a potential localization effort will radically rewrite the strings in any case. I'll include a better check in a PR I'll send soon.

@apyrgio apyrgio added this to the 0.6.1 milestone Apr 30, 2024
apyrgio added a commit that referenced this issue Apr 30, 2024
We have recently [1] changed the name of the Dangerzone application to
capital-case "Dangerzone", but this breaks our PDF viewer detection
logic. Adjust our check to exclude Dangerzone from the list.

Fixes #790

[1]: See commit 3d426ed
@deeplow
Copy link
Contributor

deeplow commented May 3, 2024

Damn. Great example of how fixing a bug created another one.

@naglis
Copy link
Contributor Author

naglis commented May 3, 2024

Damn. Great example of how fixing a bug created another one.

IIUC by refactoring _find_pdf_viewers a bit (e.g. to allow controlling the search paths) we could write tests for it to check Dangerzone is excluded/some example application is found.

@deeplow
Copy link
Contributor

deeplow commented May 3, 2024

Honestly I think @apyrgio approach is enough (5f2b632) for now. At least until localization comes along as he states.

@apyrgio apyrgio closed this as completed in 0557e34 May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants