Review Request 123294: Fix check to see if we are the active window before showing notification in polkit agent
David Edmundson
david at davidedmundson.co.uk
Wed Apr 8 15:13:25 UTC 2015
> On April 8, 2015, 2:08 p.m., Martin Gräßlin wrote:
> > AuthDialog.cpp, lines 320-321
> > <https://git.reviewboard.kde.org/r/123294/diff/1/?file=360483#file360483line320>
> >
> > one event cycle might not be sufficient to figure out whether it's the active window.
> >
> > And honestly I have no idea how one could get the information whether the window manager processed it in a reliable way. What might be the most reliable is to wait for the window being added to KWindowSystem::stackingOrder (there's a changed signal). It would give at least the hint that the window manager processed the window being shown.
The check to see if we're on top is currently broken. It looks at kwindowsystem::activeWindow which indirectly comes from looking a property on the root window. KWindowsystem only updates changes from the root window system every event loop. So we can't use the value directly after we've opened our dialog.
Given we're calling forceWindowActive in policykitlistener when we show, so I don't actually need to see what the window manager has to say as I know we'll always be on top. To some extent we don't need this notification code at all, and I could just remove it.
However, for the case that forceWindowActive doesn't work (such as running on Wayland) I left the code in.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123294/#review78671
-----------------------------------------------------------
On April 8, 2015, 1:16 p.m., David Edmundson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123294/
> -----------------------------------------------------------
>
> (Updated April 8, 2015, 1:16 p.m.)
>
>
> Review request for Plasma.
>
>
> Repository: polkit-kde-agent-1
>
>
> Description
> -------
>
> KWindowSystem::activeWindow result is cached, we need to process an
> event loop for it to update.
>
>
> Diffs
> -----
>
> AuthDialog.h 561c6a0
> AuthDialog.cpp 16ee64d
>
> Diff: https://git.reviewboard.kde.org/r/123294/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> David Edmundson
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150408/d2963691/attachment.html>
More information about the Plasma-devel
mailing list