Review Request 128571: Add Qt::Dialog to default flags to make QXcbWindow::isTransient() happy.

[Eike Hein]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128571/
-----------------------------------------------------------

(Updated Aug. 2, 2016, 10:20 a.m.)


Status
------

This change has been marked as submitted.


Review request for Plasma and Marco Martin.


Changes
-------

Submitted with commit 433b6794fe8c08fee91dd8d307604ae0207d14c5 by Eike Hein to branch master.


Bugs: 366278
    http://bugs.kde.org/show_bug.cgi?id=366278


Repository: plasma-framework


Description
-------

Panel popup dialogs along with other applications of Plasma::Dialog
(e.g. Kicker's submenus) currently don't correctly set WM_TRANSIENT_FOR
to the id of their parent window on X11. This causes them to interact
badly with auto-hide panels which do not set struts, e.g. Kicker's
submenus open behind the panel.

Internally, Dialog makes calls to QWindow::setFlags when its window
type is "Normal" (the default) and otherwise uses KWindowSystem's
setType. (Kicker's subclass, SubMenu, does an additional
KWindowSystem::setType call for NET::Menu). Neither CompactApplet
nor Kicker change their Dialog's 'type' property to anything else,
so their dialogs are "Normal", thus going the setFlags route with
no setType calls.

Dialog also sets its transient parent to the window the visualParent
item is inside of.

Now here is where things break down: QXcbWindow will update
WM_TRANSIENT_PARENT for in show(), but only when an inline function
called isTransient returns true. isTransient decides this based on
window type, which is determined from the flags set with
QWindow::setFlags. Calls to KWindowSystem::setType would have no
bearing on this; there seems to be no mapping back from external
state. This is why setting Dialog.type to e.g. "DialogWindow" does
nothing.

This patch takes the route of adding Qt::Dialog to the starting
flags - after all Dialog is a dialog. That means isTransient()
will consider the window to be a transient and those
setTransientParent calls Dialog does will not be ignored. In the
case of CompactApplet, no further calls to KWS::setType are done;
Kicker continues to call it with NET::Menu to get desired window
manager behavior.

In light testing everything still seems too work, with the added
benefit of fixing:
BUG:366278

That said, the weird mess of setFlags and setType and state in the
windowing system vs. Qt is horrible and sad.


Diffs
-----

  src/plasmaquick/dialog.cpp eb9fc79 

Diff: https://git.reviewboard.kde.org/r/128571/diff/


Testing
-------


Thanks,

Eike Hein

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160802/138e1e47/attachment-0001.html>