Closed naglis closed 5 months ago
Nice find. I haven't tested, but I wonder if this also fixes https://github.com/freedomofpress/dangerzone/issues/135.
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":
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.
@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 |
---|---|
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 |
---|---|
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.
Thanks you very much for the stellar bug report :star_struck: . 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.
In any case, I'd suggest adding the
setApplicationName("dangerzone)
call, and moving those two calls inApplication.__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.
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.
Cool, I'm gonna add this extra setting then, run some tests, and then merge your PR (unless you're already changing stuff).
Thanks, please go ahead if you'd like to do it more quickly :+1: 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; to inspect the app ID values of currently running application windows under sway runswaymsg -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:
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.