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

Bump XDG shell stable protocol from version 1 to 5 #2784

Merged
merged 7 commits into from Mar 14, 2023
Merged

Bump XDG shell stable protocol from version 1 to 5 #2784

merged 7 commits into from Mar 14, 2023

Conversation

wmww
Copy link
Contributor

@wmww wmww commented Jan 13, 2023

Fixes #2778. Not sure if more things need to be implemented before we ship it. Definitely needs manual testing and WLCS coverage.

@wmww wmww requested a review from AlanGriffiths March 9, 2023 20:34
@wmww wmww marked this pull request as ready for review March 9, 2023 20:34
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

This seems usable as is, but leaves a number of gaps.

There should be a plan (backlog items, issues, follow-up PRs) to resolve these before landing.

bors delegate

Comment on lines +101 to +102
# Not yet implemented, see https://github.com/MirServer/mir/issues/2861
XdgPopupTest.when_parent_surface_is_moved_a_reactive_popup_is_moved
Copy link
Contributor

Choose a reason for hiding this comment

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

There's some overlap between "reactive" popups and "satellites" that we should consider in the window management code.

Comment on lines 476 to +478
Destroy the xdg_surface object. An xdg_surface must only be destroyed
after its role object has been destroyed.
after its role object has been destroyed, otherwise
a defunct_role_object error is raised.
Copy link
Contributor

Choose a reason for hiding this comment

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

We've recently been discussing this condition (and we are not raising this error).

Comment on lines +536 to +539
will raise an invalid_size error. When applied, the effective window
geometry will be the set window geometry clamped to the bounding
rectangle of the combined geometry of the surface of the xdg_surface and
the associated subsurfaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

we are not raising this error

Comment on lines +879 to +885
void mf::XdgPositionerStable::set_parent_size(int32_t parent_width, int32_t parent_height)
{
(void)parent_width;
(void)parent_height;
// TODO
log_warning("xdg_positioner.set_parent_size not implemented");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We ought to have a plan for cleaning up these "TODO"s

Comment on lines +887 to +892
void mf::XdgPositionerStable::set_parent_configure(uint32_t serial)
{
(void)serial;
// TODO
log_warning("xdg_positioner.set_parent_configure not implemented");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We ought to have a plan for cleaning up these "TODO"s

@AlanGriffiths
Copy link
Contributor

bors merge

@bors bors bot merged commit e33d2e4 into main Mar 14, 2023
40 checks passed
@bors bors bot deleted the xdg-shell-v5 branch March 14, 2023 13:16
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.

Time to refresh xdg-shell implementation
2 participants