Initial support for popup window handling
ClosedPublic

Authored by graesslin on Mar 25 2017, 5:49 PM.

Details

Summary

So far KWin did not properly handle popup windows. That is when a popup
surface got created and a click outside the surface happened KWin did not
send out the popupDone Wayland event.

This change makes KWin aware of whether a surface is a popup and tracks
through a new PopupInputFilter whether there are popup windows. In case
there are popups the new filter waits for mouse press events and cancels
the popups if the press does not happen on any surface belonging to the
same client. To quote the relevant section of the Wayland documentation:

The popup grab continues until the window is destroyed or a mouse
button is pressed in any other client's window. A click in any of the
client's surfaces is reported as normal, however, clicks in other
clients' surfaces will be discarded and trigger the callback.

So far the support is still incomplete. Not yet implemented are:

  • support xdg_shell popup windows
  • verifying whether the popup is allowed to be a popup
  • cancel the popup on more global interactions like screen lock or kwin effect

BUG: 366609
FIXED-IN: 5.10

Test Plan

Auto test and manual testing with QtWayland client

Diff Detail

Repository
R108 KWin
Branch
popup-filter
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin created this revision.Mar 25 2017, 5:49 PM
Restricted Application added a project: KWin. · View Herald TranscriptMar 25 2017, 5:49 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

I tried it and compiles fine. Some popups don't yet go away though:

  • Click on window decoration of window with the popup.
  • Panel and launcher popups, click on other part of plasmashell.
broulik added inline comments.
popup_input_filter.cpp
63

constLast()

A thing to the future might be to consider blocking hover events for other clients, too? On X while a menu is opened, buttons in other clients won't highlight as the menu grabs. On Wayland they would still highlight.

In D5177#97858, @subdiff wrote:

I tried it and compiles fine. Some popups don't yet go away though:

  • Click on window decoration of window with the popup.
  • Panel and launcher popups, click on other part of plasmashell.

Thanks for testing. I don't really understand the two cases where it doesn't work correctly. I assume they are different issues.

A thing to the future might be to consider blocking hover events for other clients, too? On X while a menu is opened, buttons in other clients won't highlight as the menu grabs. On Wayland they would still highlight.

I'm not sure. I read the documentation about 20 times to understand what is expected and did not find any hint on whether mouse motion should be constrained to the popup. The only hint is about the buttons. So I tried to stay as close to the documentation as possible.

Concerning whether or not it should update: I'm not sure. On X11 it's due to the pointer grab, but is that a feature or a bug?

graesslin marked an inline comment as done.Mar 27 2017, 2:50 PM
graesslin updated this revision to Diff 12861.Mar 27 2017, 2:52 PM

last -> constLast

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptMar 27 2017, 2:52 PM

Concerning whether or not it should update: I'm not sure. On X11 it's due to the pointer grab, but is that a feature or a bug?

I think it's a side-effect of the pointer grab which quite effectively communicates that clicking the button wouldn't trigger it because a menu's open. Fwict it's also the case on Wayland for exec()d menus within the same client.

Concerning whether or not it should update: I'm not sure. On X11 it's due to the pointer grab, but is that a feature or a bug?

I think it's a side-effect of the pointer grab which quite effectively communicates that clicking the button wouldn't trigger it because a menu's open.

I think we have better ways to communicate it. E.g. we could change the cursor to indicate that it will close the popup. Or darken all other windows. That we are not restricted any more is something I quite like as it allows scrolling to work in other windows even if a context menu is open.

Fwict it's also the case on Wayland for exec()d menus within the same client.

you mean that is how qt handles it internally. Other toolkits might handle it differently ;-)

broulik accepted this revision.Apr 7 2017, 2:09 PM
This revision is now accepted and ready to land.Apr 7 2017, 2:09 PM
romangg accepted this revision.Apr 7 2017, 2:10 PM

Ok, let's go for it and handle cases not yet working perfectly later.

This revision was automatically updated to reflect the committed changes.