Review Request 112185: Fix KManagerSelection and re-enable unit test

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112185/#review38333
-----------------------------------------------------------

Ship it!


Many thanks for taking care of this.


tier1/kwindowsystem/src/kmanagerselection.cpp
<http://git.reviewboard.kde.org/r/112185/#comment28349>

    that's still a weird if() then, if it does nothing in the body.
    Why not just call owner(); ?


- David Faure


On Aug. 21, 2013, 1:33 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112185/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2013, 1:33 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Description
> -------
> 
> Re-enable unit test for kmanagerselection
> 
> The testLostOwnership is set to expected fail as that one used
> X event queue functionality not available in XCB.
> 
> The other tests are working.
> 
> Always emit the claimedOwnership() signal if we have the selection
> 
> There was a logical error introduced during the porting to XCB. There
> is no requirement for the old owner to destroy its selection window.
> Which means we should not emit the failed signal if the window has not
> been destroyed in time.
> 
> This is especially important for the situation that another Selection
> Owner of the same X client takes over as the SelectionClear event is
> not delivered in that case and thus the destroy never happens and the
> new owner thinking it failed.
> 
> The timer is in fact only relevant for the case that the kill option
> is set. We want to give the old owner the time to clean up itself
> instead of forcefully killing (e.g. window manager --replace).
> 
> Filter out events only relevant to the KSelectionOwner
> 
> No need to pass the events about the selections we are monitoring to Qt.
> 
> Ensure the KSelectionOwner destroys its window in the dtor
> 
> Release() might not destroy the window, so better make sure that
> the window gets destroyed.
> 
> Re-add code for client message handling in KSelectionWatcher
> 
> While the code looks like being no-op, it is in fact not, because in
> one path it calls owner() which might update the selection owner and
> emits newOwner signal. Updating the owner information has to happen
> in response to handling the client message.
> 
> Need to declare metatype for xcb_window_t
> 
> This fixes not delivered signals - e.g. newOwner.
> 
> 
> Diffs
> -----
> 
>   tier1/kwindowsystem/autotests/CMakeLists.txt 614b852 
>   tier1/kwindowsystem/autotests/kmanagerselectiontest.h 15858dc 
>   tier1/kwindowsystem/autotests/kmanagerselectiontest.cpp 887cb3f 
>   tier1/kwindowsystem/src/kmanagerselection.h 8df8ead 
>   tier1/kwindowsystem/src/kmanagerselection.cpp d7453c8 
> 
> Diff: http://git.reviewboard.kde.org/r/112185/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130822/5830ce74/attachment.html>