fix(wayland): [Workaround] disable WebGTK legacy rendering tech#2124
fix(wayland): [Workaround] disable WebGTK legacy rendering tech#2124M3TIOR wants to merge 5 commits intosafing:developmentfrom
Conversation
Disabled incompatible WebGTK rendering pipelines outside known X11 sessions.
For a more detailed writeup on the issue see:
flusterIO/fluster#11
This project inherits the same bugs thanks to using Taury and depending on
the system's webview (in this case WebGTK) implementation.
|
|
📝 WalkthroughWalkthroughAdds a new Linux UI wrapper script and installs it via updated Tauri packaging; postinst now conditionally relocates an existing Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
desktop/tauri/src-tauri/tauri.conf.json5 (1)
96-123:⚠️ Potential issue | 🔴 CriticalRPM
filessection is missing theui-wrappermapping — RPM postinst will break.The
ui-wrapperwas added todeb.files(line 82) but not torpm.files. Since both package types share the samepostinstscript (which runschmod +x /usr/lib/portmaster/ui-wrapperandln -srf … ui-wrapper …), RPM installs will fail because the file won't exist on disk.Proposed fix — add the mapping to rpm.files
"/var/lib/portmaster/intel/news.yaml" : "intel/news.yaml", + // Scripts + "/usr/lib/portmaster/ui-wrapper": "../../../packaging/linux/ui-wrapper", + // Shortcut "/etc/xdg/autostart/portmaster.desktop": "../../../packaging/linux/portmaster-autostart.desktop"
🤖 Fix all issues with AI agents
In `@packaging/linux/postinst`:
- Around line 66-68: Guard the move of the original binary and make the symlink
idempotent: before calling mv /usr/bin/portmaster
/usr/lib/portmaster/portmaster-ui ensure the source is a regular file (e.g. test
-f /usr/bin/portmaster) so the mv only runs when there is something to move, and
change the ln invocation creating /usr/bin/portmaster to use -srf (or at least
-sr -f) so creating the symlink to /usr/lib/portmaster/ui-wrapper is forced and
won’t fail if the symlink already exists; keep the chmod +x
/usr/lib/portmaster/ui-wrapper step as-is.
In `@packaging/linux/ui-wrapper`:
- Line 17: Fix the typo in the shell comment that currently reads
"compatability" by replacing it with "compatibility" in the line starting with
"# Assume worst case scenario for compatability with Wayland in" so the comment
correctly reads "compatibility".
- Line 5: The conditional uses incorrect redirection in the line `if command -v
"loginctl" 1>&2 2>/dev/null; then` which causes stdout to leak to stderr; change
the redirection order so both stdout and stderr are discarded (e.g., redirect
stdout to /dev/null then redirect stderr to stdout) by replacing that
redirection sequence with the standard form that silences both streams for the
`command -v "loginctl"` check.
- Around line 1-22: The script uses a shebang with -e which causes the script to
exit if the loginctl invocation fails, preventing the else fallback; change to
remove the -e behavior and defensively capture the session type into a variable
(e.g., SESSION_TYPE="$(loginctl -P Type show-session auto 2>/dev/null || true)")
then test SESSION_TYPE in the case statement, and keep the single export/exec
path that sets WEBKIT_DISABLE_DMABUF_RENDERER and
WEBKIT_DISABLE_COMPOSITING_MODE before exec /usr/lib/portmaster/portmaster-ui
"$@" so failures in loginctl fall through to the Wayland-compatible defaults
rather than aborting the script.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packaging/linux/postinst`:
- Line 69: Fix the typo in the shell comment that reads "# Checksum command
nomrally dumps other info besides the actual checksum" by changing "nomrally" to
"normally" so the comment reads "# Checksum command normally dumps other info
besides the actual checksum" (update the comment in the postinst script where
that line appears).
- Around line 68-77: The checksum guard is fragile and can mis-trigger when
/usr/bin/portmaster is missing; replace it with a robust file-type and existence
check: detect if /usr/bin/portmaster exists and is a symlink pointing to
/usr/lib/portmaster/ui-wrapper (use readlink -f or test -L plus comparison) and
only skip the mv when that is true; otherwise, if /usr/bin/portmaster exists and
is not the symlink, move it to /usr/lib/portmaster/portmaster-ui (ensure you
test -e before invoking mv to avoid errors). Ensure you remove or stop using the
cksum() function and update the conditional that currently compares "$(cksum
/usr/bin/portmaster)" to "$(cksum /usr/lib/portmaster/ui-wrapper)" to the new
existence/symlink check.
2 participants
Disabled incompatible WebGTK rendering pipelines outside known X11 sessions. For a more detailed writeup on the issue see:
flusterIO/fluster#11
This project inherits the same bugs thanks to using Taury and depending on the system's webview (in this case WebGTK) implementation.
NOTE: This is untested code, I couldn't run a test build because your build system requires I be authenticated to Github for some reason! Also, it seemed like it scanned my LAN for SSH targets? I'm going to assume good faith but I know that's a bad idea. Really bums me out I can't validate this build for you before pushing it.
On my own rig I've done a temporary fix by wrapping the .desktop UI execution lines with a call to /usr/bin/env. But the below method is more maintainable in the event you encounter systems which run another system session like Ubuntu tried to implement at one point. Hopefully my POSIX shell script doesn't contain any errors.
Summary by CodeRabbit
Chores
Stability