Differential D16890

[systemtray] Keep config entries after modifying
ClosedPublic
Actions

Authored by anthonyfieroni on Nov 15 2018, 8:00 AM.

Details

Reviewers
broulik
davidedmundson
ngraham
Group Reviewers
Plasma
Commits
R120:0885170daf56: [systemtray] Workaround component creation
Summary

It still has one problem, after configuration is changed in Entries MouseArea index is invalid, i can't understand why, @broulik ?

BUG: 390890

Test Plan
  1. Right button on systemtray
  2. Check / uncheck extra imtem
  3. Click on Entries
  4. Show / hidden some of them
  5. Return to General

again 2
again 4

All good, but after close configuration dialog clicking on item in systray is an invalid index.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
anthonyfieroni created this revision.Nov 15 2018, 8:00 AM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 15 2018, 8:00 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
anthonyfieroni requested review of this revision.Nov 15 2018, 8:00 AM
anthonyfieroni added a comment.Nov 15 2018, 1:26 PM

Weird. If configuration window stays open, clicks in systray work correct, when it's dismissed clicks are incorrect.

broulik added a comment.Nov 15 2018, 4:53 PM

I think that's a Qt bug, i did an investigation in another systray bug report but can't find it right now

ngraham added a subscriber: ngraham.Nov 18 2018, 3:50 AM
In D16890#359757, @anthonyfieroni wrote:

Weird. If configuration window stays open, clicks in systray work correct, when it's dismissed clicks are incorrect.

That's a separate bug: https://bugs.kde.org/show_bug.cgi?id=393630

ngraham accepted this revision.Nov 18 2018, 3:54 AM

This does fix 390890 for me though! Everything else still seems to work.

This revision is now accepted and ready to land.Nov 18 2018, 3:54 AM
davidedmundson added a comment.Nov 18 2018, 5:11 PM

I don't understand.

The only usage of plasmoidItemComponent is in " Containment.onAppletAdded:", so how is this going to make any difference?

anthonyfieroni added a comment.Nov 18 2018, 6:56 PM

But for some reason, on second Entries show warnings says: cannot call createObject on null object. Looks like it is treated as local object or something.

trmdi added a subscriber: trmdi.Nov 28 2018, 1:35 PM
In D16890#366015, @anthonyfieroni wrote:

To commit it @mart @broulik ?

I think that is not the root cause. You should fix the root cause instead of just a part of it.
See my comment on https://bugs.kde.org/show_bug.cgi?id=393630#c25

anthonyfieroni added a comment.Nov 28 2018, 1:54 PM

Yep, it's Qt bug.

ngraham added a comment.Nov 28 2018, 2:48 PM

If it's a Qt bug, our options are to fix it upstream, or work around it here with a comment in the code indicating the workaround and the Qt bug report. Can you add that?

davidedmundson accepted this revision.Jan 14 2019, 5:30 PM

The bug has been reported before!

https://bugreports.qt.io/browse/QTBUG-71238 / https://bugreports.qt.io/browse/QTBUG-72004

Ignore the title and context, read the comment from Kai about the component without a parent getting cleaned up.

That moves this from working round an "unknown", to working round a "known" issue.
Seems a tiny difference, but means a lot to whether I accept it.

ngraham added a comment.Jan 14 2019, 5:37 PM

@anthonyfieroni can you add a comment in the code that indicates the Qt issues this is working around?

We've got three days until 5.15 tagging. Let's try to get this in!

Closed by commit R120:0885170daf56: [systemtray] Workaround component creation (authored by anthonyfieroni). · Explain WhyJan 14 2019, 6:14 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 49467

View Options

applets/systemtray/package/contents/ui/main.qml

Loading...