> On July 15, 2016, 7:42 a.m., David Rosca wrote: > > Does it work with Qt < 5.6.1 now? > > Martin Gräßlin wrote: > That would be before your change? I guess no. Btw. I read your code > through today in an extremely detailed manner and I'm 100 % sure, that it's > absolutely correct. Good work! > > Eike Hein wrote: > No idea: I only have Qt 5.7. But I don't think this code makes it any > worse, since we're keeping the "we need this for <Qt 5.6.1" line intact, and > the other change shouldn't make it /worse/. > > David Rosca wrote: > I think it would not have the skip taskbar state at all, so the popup > will be always visible in task bar. The reason is that setState is now called > only before showEvent and PlatformSurfaceCreated and Qt < 5.6.1 clears the > "unknown" states in showEvent.
> since we're keeping the we need this for <Qt 5.6.1 Actually no, probably my comment was misleading but for < 5.6.1 it is needed to set the state *after* showEvent. We're now only keeping the line that makes it work correctly with >= 5.6.1 (at least for some :D) - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128457/#review97421 ----------------------------------------------------------- On July 15, 2016, 7:38 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, 7:38 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 > ----- > > 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
