not sure if that is the right "parent" that has
to be set, set the transient for surfaces that
have setParentOf of an imported set
Details
Details
- Reviewers
graesslin - Group Reviewers
Plasma - Commits
- R108:735fcc6e954f: Make use of foreign protocol
R108:6faad34cdfd3: [WIP] make use of foreign protocol
m_XdgForeign->transientFor(surface()) finds the surface is expected,
not sure how to test it further
Diff Detail
Diff Detail
- Repository
- R108 KWin
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Looks good to me.
not sure how to test it further
If you did want to autotest it, it'd be in the integration folder; it uses the real shell_client with a fake workspace.
If you want a real world use-case for this:
kde-polkit-agent-1 has a special bit of code to pass a WId (currently an int) to the agent before making a polkit call.
kwallet also passes a WId in the open() call. Both will require lots of API changes though :(
| shell_client.cpp | ||
|---|---|---|
| 1401 | I would delegate this to waylandServer, so that there is no need to pass the foreign global to each ShellClient. | |
| 1485 | This connect could be done in waylandServer then there would not be a reason to have this method at all. | |
| shell_client.h | ||
| 129 ↗ | (On Diff #18689) | Please drop the UnstableInterface. |
| 212 ↗ | (On Diff #18689) | No need to explicitly set the QPointer to nullptr. |
| wayland_server.cpp | ||
| 167 | Why is the xdgForeign passed to the client? | |
| shell_client.cpp | ||
|---|---|---|
| 1401 | how could it be done as addtransient/transientfor/settransient are all protected? | |
Comment Actions
one way i could do, is to add setters for xdgshell to set transient from there, to then use its tracking, it would depend from xdgshell existing, but that's what the protocol says anyways
| shell_client.cpp | ||
|---|---|---|
| 1401 | what I was thinking of is something like: SurfaceInterface *WaylandServer::findForeignForSurface(SurfaceInterface *surface)
{
return m_XdgForeign->transientFor(surface);
}and here a simple: if (!s) {
s = waylandSurface()->findForeignForSurface(surface());
}but I think your question is more about how to do the connect: just make waylandServer forward the signal, so that ShellClient can connect to it. What I don't want is that the global is passed to ShellClient - we don't do that for any other global in ShellClient. Normally it always goes through WaylandServer. There are very few exceptions to it where a class has direct access to a global. | |
Comment Actions
here a different approach, if the other one is better, i can get back to it and add WaylandServer::findForeignForSurface
Comment Actions
it still does some weird hoops to set it modal to the parent, it should probably be set somehow client side but doesn't seem to be yet anything in xdgshell?
Comment Actions
What I absolutely don't understand is why you want to make it modal. I don't see a reason for this to be modal and the X11 variant where this was used is not modal. E.g. kwallet is not modal, polkit is not modal and a file dialog through a flatpack portal should not be modal .
Furthermore our Wayland implementation does not even support any modal windows yet:
AbstractClient *ShellClient::findModal(bool allow_itself)
{
Q_UNUSED(allow_itself)
return nullptr;
}| shell_client.h | ||
|---|---|---|
| 64–66 ↗ | (On Diff #18766) | I don't see this new method being used. |
| wayland_server.cpp | ||
| 166–173 | why the setModal? Why should a foreign transient be modal? | |
| 174 | as an alternative feel free to make ShellClient::setTransient public (it's only private because nothing used it so far) and do: if (childClient && childClient->surface() == child) {
childClient->setTransient();
} | |
| wayland_server.h | ||
| 198 | why add the parent if it's never used? | |
Comment Actions
Ok a file open dialog might be modal, but also not everywhere (e.g. it is in Firefox, but isn't in libreoffice)
Comment Actions
There's a major pitfall to inter-process modality:
In case of a persistent transient (used by several clients), the leader may loose the focus access "forever" (because the modal window remains)
Thus and in general, transients should certainly not be unconditionally modal, but the ability to control this attribute to the WM will be useful.
If client A crucially relies on input in client B to proceed, it will typically reject internal focus distribution and input handling.
This is complex enough if the client spawns a nested event loop and tries to pass the focus on to the foreign window, but in a multi-process case, client A might simply block for the I/O to gain this behavior.
In this case a "dead" window might receive the input focus.
Also the modality controls the forced visibility of the transient along the leader.
Comment Actions
- use xdgshell setTransientFor
- use WaylandServer::findForeignTransientForSurface
- drop unused code