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

Set the desktop filename of the Qt application #774

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

naglis
Copy link
Contributor

@naglis naglis commented Apr 9, 2024

Currently, the app ID of the Dangerzone GUI application when running under Wayland is python3, which is not very useful if one wants to automate some action related to the Dangerzone application window (e.g. to always start Dangerzone window in floating mode under Sway WM; to inspect the app ID values of currently running application windows under sway run swaymsg -t get_tree).

Setting the desktop filename property also sets the app ID of the application under Wayland. According to Qt documentation, the property value should be the name of the application's .desktop file but without the extension.

Qt documentation also states:

This property gives a precise indication of what desktop entry > represents the application and it is needed by the windowing system to retrieve such information without resorting to imprecise heuristics.

Therefore I also think that setting this property is needed to display the correct application name and icon (taken from the .desktop entry) when running under certain windowing systems (like Wayland) (see also #402, after testing locally under Gnome I believe this should fix it).

PR questions

According to Qt 5 docs this property was introduced in Qt 5.7. IIUC PySide2 is Qt 5.12+, so it should be ok to set it without wrapping it in try/except AttributeError? Please correct me if I am mistaken.

Also, I am not sure if perhaps Application's __init__ is the more suitable/correct place for setting it. Please let me know if it should be moved there.

@deeplow
Copy link
Contributor

deeplow commented Apr 23, 2024

Nice find. I haven't tested, but I wonder if this also fixes #135.

@apyrgio
Copy link
Contributor

apyrgio commented Apr 24, 2024

I have some questions here, because I tried to reproduce this on Fedora 39 (with PySide6, Gnome and Wayland) and Ubuntu 24.04 (with PySide2, Gnome and XWayland), but the window title correctly shows "Dangerzone":

  1. Can you tell me a bit about your configuration? I see that you use Sway, but what's your OS?
  2. Have you tested if the window title also shows incorrectly with the stock window manager of your OS? If not, that's ok, I can take a look.
  3. Are we talking about running Dangerzone from source, or installing it from a package?

According to Qt 5 docs this property was introduced in Qt 5.7. IIUC PySide2 is Qt 5.12+, so it should be ok to set it without wrapping it in try/except AttributeError? Please correct me if I am mistaken.

Yeah, we're good. Ubuntu Focal is our oldest supported platform, and even that has Qt 5.12.

@naglis
Copy link
Contributor Author

naglis commented Apr 25, 2024

@apyrgio, thank you for testing. Sorry, I think I did not include all the information to make it clear.

Can you tell me a bit about your configuration? I see that you use Sway, but what's your OS?

Sure, I am using Arch Linux.

Have you tested if the window title also shows incorrectly with the stock window manager of your OS? If not, that's ok, I can take a look.

Arch Linux does not really have a "stock" window manager, generally you install and use whichever you prefer. I have tested it under Gnome 46 and Sway 1.9 using Wayland 1.22. Also, please note that the issue with the application name I've mentioned is not the window title, but how the application name/icon is displayed in Gnome's window switcher/Activities overview (see #402 for more information).

Are we talking about running Dangerzone from source, or installing it from a package?

Both (note that Arch Linux does not have an official package, I am using a modified PKGBUILD to get 0.6.0 instead of 0.5.1 from the AUR). Although I don't think that setting the desktop filename would have an effect on application name/icon when running Dangerzone from source while Dangerzone is not also installed as a package as Gnome would not be able to find the matching .desktop entry.


The issue I had initially was not being able to control how Dangerzone window is displayed when it runs under Sway. Sway is a tiling window manager, so by default, the window of an application as it starts is maximized to take up the whole screen (ignoring any window width/height preferences set by the application). I wanted Dangerzone window to be displayed in the size as set in the application, as it does not benefit from starting in full screen mode and looks better when not maximized. To do that under Sway, one can assign certain applications, to start in floating mode and not maximized. To be able to identify an application under Sway/Wayland, the app_id property of the window can be used (similar to using xprop to find the class or wm_name attributes under X11). When trying to find the app_id of Dangerzone using swaymsg -t get_tree I've found that it is python3 when starting both from source and from the installed package. This value is not very useful, as it is generic and does not uniquely identify Dangerzone. To inspect Wayland window properties under Gnome, one can follow this answer on AskUbuntu. Here are the results I get:

Without setDesktopFileName With setDestkopFileName
Gnome Looking Glass output for Dangerzone window when not using setDesktopFilename() Gnome Looking Glass output for Dangerzone window when using setDesktopFilename()

I've found that setting the desktop filename using setDesktopFileName also sets the Wayland window's app ID to that value. Since according to Qt documentation the value passed to setDesktopFileName should be the application's .desktop filename without the extension, this also allows to uniquely identify the application via its Wayland window's app ID.

While searching for potentially related issues on Dangerzone's issue tracker I've stumbled upon #402. I was able to reproduce it locally under Gnome and have found that setting the desktop entry filename (and thus tha app ID) to press.freedom.dangerzone (the Dangerzone's .desktop entry filename) allows Gnome to correctly match the application to its desktop entry and display the correct application name/icon. Here are some screenshots for what I mean (notice the icon under the Dangerzone window/the icon and the application name tooltip in the dock at the bottom):

Without setDesktopFileName With setDestkopFileName
Gnome Activity Overview when not using setDesktopFileName Gnome Activity Overview when using setDesktopFileName

Therefore, I believe that setting the desktop entry filename might also be the fix for #402. Could you test if the Dangerzone application name/icon in Gnome's Activity Overview (pressing the Super key) is correctly displayed on you side and, if not, if setting the desktop filename fixes it?

Here are some references to the application name/icon issue from other projects:

Please let me know if I've missed some details or if something is not clear.

@apyrgio
Copy link
Contributor

apyrgio commented Apr 25, 2024

Thanks you very much for the stellar bug report 🤩 . The "looking glass" feature of Gnome was something I didn't know about. I did check on a Fedora 39 VM, and here's what I saw:

Dangerzone
  wmclass: __init__.py
  <untracked>

With your setDesktopFileName suggestion, the application type is now recognized:

Dangerzone
  wmclass: __init__.py
  app: press.freedom.dangezone.desktop

And here's where it gets interesting, and probably completely fixes #402. I used setApplicationName("dangerzone") (see docs), and my window properties are finally correct:

Dangerzone
  wmclass: dangerzone
  app: press.freedom.dangezone.desktop

I'm not sure why your wmclass property is different, but probably has to do with the way the window managers autocomplete missing properties.

In any case, I'd suggest adding the setApplicationName("dangerzone) call, and moving those two calls in Application.__init__(). Are you ok with this change?

In that case, let's also add a "Fixes #402" in the commit message.

@naglis
Copy link
Contributor Author

naglis commented Apr 25, 2024

In any case, I'd suggest adding the setApplicationName("dangerzone) call, and moving those two calls in Application.__init__(). Are you ok with this change?

Sure, although I am wondering if it should be setApplicationName("Dangerzone") instead, i.e. start with upper-case for consistency? As that is the case in the .desktop entry and also I saw it mentioned in this comment.

@apyrgio
Copy link
Contributor

apyrgio commented Apr 25, 2024

I had the same question. The Qt6 docs on applicationName shed more light into this:

The application name is used in various Qt classes and modules, most prominently in QSettings when it is constructed using the default constructor. Other uses are in formatted logging output (see qSetMessagePattern()), in output by QCommandLineParser, in QTemporaryDir and QTemporaryFile default paths, and in some file locations of QStandardPaths. Qt D-Bus, Accessibility, and the XCB platform integration make use of the application name, too.

(the respective Qt5 docs have less to say, so there may be some implementation difference)

To me, this reads like a case for a literal dangerzone, not a user-facing string ("Dangerzone"). Also, another thing that caught my eye is:

If not set, the application name defaults to the executable name.

In our case this is dangerzone as well.

@apyrgio
Copy link
Contributor

apyrgio commented Apr 25, 2024

Cool, I'm gonna add this extra setting then, run some tests, and then merge your PR (unless you're already changing stuff).

@naglis
Copy link
Contributor Author

naglis commented Apr 25, 2024

Thanks, please go ahead if you'd like to do it more quickly 👍 If not, I'll try to update it later today/tomorrow.

Currently, the app ID of the Dangerzone GUI application when running
under Wayland is `python3`, which is not very useful if one wants to
automate some action related to the Dangerzone application window (e.g.
to always start Dangerzone window in floating mode under Sway WM).

Setting the desktop filename property also sets the app ID of the
application under Wayland. According to Qt documentation[1], the
property value should be the name of the application's .desktop file but
without the extension.

Qt documentation also states:

> This property gives a precise indication of what desktop entry
> represents the application and it is needed by the windowing system to
> retrieve such information without resorting to imprecise heuristics.

Therefore I also think that setting this property is needed to display
the correct application name and icon (taken from the .desktop entry)
when running under certain windowing systems (like Wayland)
(see also freedomofpress#402).

Note that this property is not enough, as we've encountered systems
where setting just the desktop file name does not alter the detected
application name by the window manager. For this reason, we also use
set the application name [2] to `dangerzone`, to remove any ambiguity.

[1]: https://doc.qt.io/qt-6/qguiapplication.html#desktopFileName-prop
[2]: https://doc.qt.io/qt-6/qcoreapplication.html#applicationName-prop

Fixes freedomofpress#402
@apyrgio apyrgio merged commit 8cdb2d5 into freedomofpress:main Apr 25, 2024
38 checks passed
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 this pull request may close these issues.

3 participants