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.

Sign up for GitHub

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

Jump to bottom

Wayland fixes #6018

Merged
merged 3 commits into from
Nov 29, 2024
Merged

Wayland fixes #6018

merged 3 commits into from
Nov 29, 2024

Conversation

aleasto
Copy link
Contributor

@aleasto aleasto commented Nov 22, 2024

Wayland fixes

@pep8speaks
Copy link

pep8speaks commented Nov 22, 2024 •
edited
Loading

Hello @aleasto! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 35:1: E402 module level import not at top of file

Comment last updated at 2024-11-27 17:56:49 UTC

@aleasto aleasto changed the title Wayland Wayland fixes Nov 22, 2024
@github-actions github-actions bot added the f42 Fedora 42 label Nov 22, 2024
@Conan-Kudo
Copy link
Contributor

@jkonecny12 this needs to be backported to rhel-10 branch once merged as well.

@M4rtinK M4rtinK added the manual testing required This issue can't be merged without manual testing label Nov 25, 2024
@jkonecny12
Copy link
Member

jkonecny12 commented Nov 26, 2024 •
edited
Loading

Tested on Workstation (with GTK UI), KDE and Xfce Live ISOs. Everything seems to be working just fine.

EDIT: Tested also in boot.iso and also works fine.

@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

Hi @aleasto our tests failed on this message:

10:26:59,558 DEBUG anaconda:anaconda: misc: GLib: #012(anaconda:2630): Gtk-#033[1;35mCRITICAL#033[0m **: #033[34m10:26:59.551#033[0m: gtk_widget_set_child_visible: assertion '!_gtk_widget_is_toplevel (widget)' failed

I'm not sure if this is critical or not, I know that we are already blocking some of these messages. Do you know if it is safe to ignore?

jkonecny12 added a commit to jkonecny12/kickstart-tests that referenced this pull request Nov 27, 2024
@jkonecny12
Another Glib error from Wayland native on Live
fad4e87 
Introduced by changes in:
rhinstaller/anaconda#6018

Ignores this message:
```
10:26:59,558 DEBUG anaconda:anaconda: misc: GLib: rhinstaller#12(anaconda:2630): Gtk-rhinstaller#33[1;35mCRITICAL#033[0m **: rhinstaller#33[34m10:26:59.551#033[0m: gtk_widget_set_child_visible: assertion '!_gtk_widget_is_toplevel (widget)' failed
```
@jkonecny12 jkonecny12 mentioned this pull request Nov 27, 2024
Closed
@jkonecny12
Copy link
Member

/kickstart-test --kstest-pr 1343 --testtype smoke

@Conan-Kudo
Copy link
Contributor

It is probably safe to ignore, if the code has the correct impact. It seems to be complaining about setting the decorations to be invisible not being possible for non-windows, which is fine (we don't want dialogs to have no decorations).

jkonecny12
jkonecny12 requested changes Nov 27, 2024
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to resolve the test issue other way than ignoring the error.

@aleasto
Hide server-side-decorations in Wayland
46f609e 
Setting decorated=false doesn't do what advertised, in Wayland at least.
Gtk3 will request server-side-decorations unless a titlebar is set.

We can set any widget as titlebar, because set_decorated(False) makes it
invisible. I'm using GtkDrawingArea as a low-overhead widget.
@aleasto
Show correct taskbar icon in Wayland
4202705 
Gdk on Wayland uses prgname as the app id.
Set it to match the desktop filename.
@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

Tested on boot.iso and Lives (Workstation, Xfce, KDE) and it looks to work fine everywhere.

jkonecny12
jkonecny12 approved these changes Nov 28, 2024
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a weird workaround but I don't see any issue with that.

@jkonecny12 jkonecny12 merged commit d19089a into rhinstaller:master Nov 29, 2024
18 of 19 checks passed
@Conan-Kudo Conan-Kudo mentioned this pull request Nov 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@M4rtinK M4rtinK M4rtinK left review comments

@Conan-Kudo Conan-Kudo Conan-Kudo left review comments

@jkonecny12 jkonecny12 jkonecny12 approved these changes

Assignees
No one assigned
Labels
f42 Fedora 42 manual testing required This issue can't be merged without manual testing port to RHEL10
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aleasto @pep8speaks @Conan-Kudo @jkonecny12 @M4rtinK