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

[Martin Gräßlin]

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


Ship it!




Good investigation!

- Martin Gräßlin


On Aug. 1, 2016, 9:47 p.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128571/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2016, 9:47 p.m.)
> 
> 
> Review request for Plasma and Marco Martin.
> 
> 
> 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/12c1275e/attachment.html>