fix: return correctly-typed D-Bus values for all standard SNI properties#141
Open
hexpwn wants to merge 1 commit intoProtonVPN:stablefrom
Open
fix: return correctly-typed D-Bus values for all standard SNI properties#141hexpwn wants to merge 1 commit intoProtonVPN:stablefrom
hexpwn wants to merge 1 commit intoProtonVPN:stablefrom
Conversation
The tray icon fails to display on desktop environments and window
managers that rely on a StatusNotifierHost bridge (e.g. snixembed on
i3wm + polybar, or any setup using an SNI-to-XEmbed proxy). The bridge
crashes immediately when it tries to read the tray icon's properties.
The StatusNotifierItem specification defines strict D-Bus types for
properties like IconPixmap (a(iiay)), ToolTip ((sa(iiay)ss)), and
WindowId (u). Previously, Get() returned a plain string ("") for any
unknown or unimplemented SNI property, and GetAll() omitted them
entirely. When a StatusNotifierHost queries these properties and
attempts to deserialize the response using the spec-defined type, it
crashes with:
GLib:ERROR:g_variant_serialised_n_children: code should not be reached
This happens because GLib's GVariant deserializer receives a string
variant where it expects a complex type like a(iiay).
This commit:
- Adds a _get_sni_properties() helper that includes all standard SNI
spec properties with their correct empty D-Bus-typed defaults
- Uses it in both Get() and GetAll() to ensure type-correct responses
- Eliminates the duplicated property dict between Get() and GetAll()
This issue was diagnosed, patched and tested by Claude Opus 4.6.
This PR is 100% vibecoded.
Ref: https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The tray icon fails to display on desktop environments and window managers that rely on a StatusNotifierHost bridge (e.g. snixembed on i3wm + polybar, or any setup using an SNI-to-XEmbed proxy). The bridge crashes immediately when it tries to read the tray icon's properties.
The StatusNotifierItem specification defines strict D-Bus types for properties like IconPixmap (a(iiay)), ToolTip ((sa(iiay)ss)), and WindowId (u). Previously, Get() returned a plain string ("") for any unknown or unimplemented SNI property, and GetAll() omitted them entirely. When a StatusNotifierHost queries these properties and attempts to deserialize the response using the spec-defined type, it crashes with:
GLib:ERROR:g_variant_serialised_n_children: code should not be reached
This happens because GLib's GVariant deserializer receives a string variant where it expects a complex type like a(iiay).
This commit:
This issue was diagnosed, and the code patched and tested by Claude Opus 4.6. This PR is 100% vibecoded, so I apologize for any nonsense.
Ref: https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/