Review Request 128457: Fix the infamous 'dialogs show up on the Task Manager' bug once more.

[David Rosca]

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




src/plasmaquick/dialog.cpp (line 1072)
<https://git.reviewboard.kde.org/r/128457/#comment65740>

    Actually, I think setting the state on Expose event was the one that made it work for < 5.6.1 and calls in other places were added by d_ed in attempt to workaround the bug (which didn't do much though).
    So I think we can remove it now from other places but keep it in Expose event.
    
    Sorry for the noise, but this is important to test with < Qt 5.6.1 (which I no longer have unfortunately).


- David Rosca


On July 15, 2016, 10:03 a.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128457/
> -----------------------------------------------------------
> 
> (Updated July 15, 2016, 10:03 a.m.)
> 
> 
> Review request for Plasma, Bhushan Shah, David Edmundson, David Rosca, Martin Gräßlin, and Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> - Initially set state (and type, and flags) in response to PlatformSurfaceCreated.
>   We know reliably this will run before the window is mapped.
> 
> - Drop the comment about removing setState() form showEvent handler, as
>   we need it to avoid state loss in this scenario:
>   <mgraesslin> the window gets mapped first time: everything is fine
>   <mgraesslin> window gets unmapped: kwin removes the state as per spec
>   <mgraesslin> qt gets change event and removes the states it doesn't care about
>   <mgraesslin> qt maps window again and sets states
>   <mgraesslin> we lost the state
>   <mgraesslin> which means we need to set the state again from our side before(!) Qt sets it
>   <mgraesslin> and before Qt maps the window
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 216769a 
>   autotests/dialogstatetest.h PRE-CREATION 
>   autotests/dialogstatetest.cpp PRE-CREATION 
>   src/plasmaquick/dialog.cpp be74067 
> 
> Diff: https://git.reviewboard.kde.org/r/128457/diff/
> 
> 
> Testing
> -------
> 
> I added debug statements to XWindowTasksModel::Private::addWindow in libtaskmanager, which runs in repsonse to KWindowSystem::windowAdded and constructs a KWindowInfo for the newly-added window. Evaluating state() & NET::SkipTaskBar there after clicking the panel button to open Kicker, without this patch, it was sometimes 'false' (i.e. not skipping the taskbar) on the initial show and on subsequent shows. Setting the state in response to PlatformSurfaceCreated seems to fix the former, and keeping the setState call in showEvent (in combination with drosca's fixes to Qt 5.6.1+) is what keeps the latter working.
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160715/38fae8ca/attachment.html>