davidedmundson added a comment.
The old code is clearly wrong.
It calls Dialog::event(event);
that will set up a shellsurface in the expose event
Then we process further and set up a shell surface in the expose event
PlasmaShellSurfaces are shared, but you still have two objects handling the
move event simultaneously, and worst still
} else if (event->type() == QEvent::Hide) {
delete m_plasmaShellSurface;
is just screaming out for a crash as dialog is sharing that same object.
We're just being very lucky at the moment.
I wouldn't want to see this reverted as-is.
But clearly we need the setRole and the setPanelTakesFocus. I missed that in
my review.
That could mean just adding an if(Dock) in Dialog.cpp , only setting those
parts in ExposeEvent here, or adding an extra virtual to configure dialog
somehow.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D10197
To: apol, #plasma, davidedmundson
Cc: graesslin, broulik, ngraham, davidedmundson, plasma-devel, ZrenBot,
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
