Differential D5851

Fix the PID updated in PlasmaWindowedModel
AbandonedPublic
Actions

Authored by davidedmundson on May 14 2017, 1:32 PM.

Details

Reviewers
None
Group Reviewers
Plasma
Summary

Fixes D5747

Test Plan

Tests now pass

Diff Detail

Repository
R127 KWayland
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
davidedmundson created this revision.May 14 2017, 1:32 PM
Restricted Application added projects: Plasma on Wayland, Frameworks. · View Herald TranscriptMay 14 2017, 1:32 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
graesslin added a subscriber: graesslin.May 14 2017, 2:23 PM
graesslin added inline comments.
src/client/plasmawindowmodel.cpp
84

In the original report I critized that we connect to a pid changed signal. That doesn't make any sense as a pid cannot change.

I think this is the wrong solution to the problem.

davidedmundson added a comment.May 14 2017, 2:51 PM

It makes absolutely no sense to have a pidChanged signal and then say clients shouldn't be using it.

Either window has no reason to have a pidChanged signal or we should connect to it. It has to be one or the other.

graesslin added a comment.May 14 2017, 4:59 PM
In D5851#109478, @davidedmundson wrote:

It makes absolutely no sense to have a pidChanged signal and then say clients shouldn't be using it.

Either window has no reason to have a pidChanged signal or we should connect to it. It has to be one or the other.

Right, the signal shouldn't exist. I overlooked that in the other review. Sorry. I pointed it out in two cases and missed the third. The signal should be removed IMHO.

davidedmundson abandoned this revision.May 15 2017, 10:47 AM

That would also be completely fine from my POV.

I should have read the original review request, I was just fixing the tests based on what was currently here.

Revision Contents
Changeset List

PathPackages
Msrc/client/plasmawindowmodel.cpp (4 lines)

Diff 14506

View Options

src/client/plasmawindowmodel.cpp

Loading...