Set a transient parent for SNI context menus
ClosedPublic

Authored by fvogt on Mar 22 2018, 6:55 PM.

Details

Summary

Those had no transient parent set, so they got displayed somewhere, most of the
time on the wrong screen.

Also pass the actual click coordinates instead of (0,0), although those are
ignored anyway.

Test Plan

Works fine for spotify and kteatime now, but certain applications
which trigger a LayoutChanged signal on the opened event have a Y offset.

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.
fvogt created this revision.Mar 22 2018, 6:55 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 22 2018, 6:55 PM
fvogt requested review of this revision.Mar 22 2018, 6:55 PM
fvogt added a comment.Mar 22 2018, 6:57 PM

The blank line deletion should be ok, but someone should test on X11 as well. I don't expect any regressions there though.

fvogt updated this revision to Diff 30248.Mar 22 2018, 8:03 PM

libdbusmenu-qt: Remove nonexistant actions directly from the menu

The getLayout response handler compares the new list of actions with the
current actions in the menu and calls deleteLater on all actions which aren't
part of the new list anymore.
Then, it adds all actions from the new list which aren't part of the menu yet.

As deleteLater only has an effect after the next event processing, the menu
still contains them together with the added actions.
This resulted in broken size calculations, as even for static menus the item
count changed during aboutToShow.

Note that this is not a proper solution for the resize issue, as the
aboutToShow handler changes the menus content for a reason, the application
is free to add/remove items at any point in time.

fvogt added a comment.Mar 22 2018, 8:04 PM

Urgh, arc. I'll split again.

fvogt updated this revision to Diff 30251.Mar 22 2018, 8:10 PM

Now for real. Please arc, cooperate.

davidedmundson added inline comments.
applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
63 ↗(On Diff #30245)

This change is unrelated, and not a change I would support.

applets/systemtray/systemtray.cpp
297

This part is all fine.

fvogt added inline comments.Mar 23 2018, 7:52 AM
applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
63 ↗(On Diff #30245)

Why not?

It is a fix for an oversight, which is currently a no-op due to another bug.

broulik added inline comments.
applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
63 ↗(On Diff #30251)

I think this change is fine. Except that the MouseArea covers the entire list item, so for hidden SNIs the app might get coordinates outside of its icon.

For context menu we ignore the coordinates anyway and place the menu relative to the icon (so it never covers it). For Activate the app might want to know where it was clicked. In any case this needs a bit of cleaning up, we pass parameters around in places where they're ignored and so on.

fvogt added inline comments.Mar 23 2018, 8:21 AM
applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
63 ↗(On Diff #30251)

I think I'll do it differently.

fvogt updated this revision to Diff 30279.Mar 23 2018, 8:24 AM

Split mouse.xy stuff into separate patch.

davidedmundson accepted this revision.Mar 23 2018, 10:43 AM
This revision is now accepted and ready to land.Mar 23 2018, 10:43 AM
This revision was automatically updated to reflect the committed changes.