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

Toplevel parent fixes, checks, and callback #1609

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

YaLTeR
Copy link
Contributor

@YaLTeR YaLTeR commented Dec 14, 2024

Several things here:

  1. Add a self-parent and a parent loop check when setting a parent, corresponding to the protocol:

    The parent toplevel must not be one of the child toplevel's descendants, and the parent must be different from the child toplevel, otherwise the invalid_parent protocol error is raised.

  2. Fix inverted logic in xdg-foreign, tested with xdg-desktop-portal-gnome dialogs.

  3. Expose an XdgShellHandler::parent_changed() callback so compositors can react to parent changes.

    The primary use is to restack surfaces as the protocol intends:

    This surface should be stacked above the parent surface and all other ancestor surfaces.

    Another use is to signal parent changes to wlr-foreign-toplevel-management.

    This makes xdg-foreign depend on XdgShellHandler. Not sure if it's doable otherwise; realistically it's required because there's no other toplevel-equivalent yet.

  4. Remove ToplevelSurface::set_parent(). This was used back in zxdg-shell days, not any more. Anyhow, exposing this publicly seems a bit strange because it can cause client protocol errors down the line from compositor actions.

The argument is the child, not the other way around.
- All places that set a parent check validity.
- Added a parent loop check.
Nothing uses it. It's also a strange function to expose publicly. Setting an
invalid (looping) parent causes a protocol error, so if the compositor sets the
parent of a toplevel without it knowing, it can result in the app subsequently
getting an unintended protocol error.
Called when the surface parent changes, either via set_parent() or via
xdg-foreign.
@PolyMeilex
Copy link
Member

PolyMeilex commented Dec 14, 2024

there's no other toplevel-equivalent yet.

XWayland is such a equivalent, but either way I don't mind this depending on xdg shell.

That's also most likely reason for set_parent's existence.

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Seems all very sensible!

@Drakulix Drakulix merged commit 68555cf into Smithay:master Dec 16, 2024
13 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