Bug 376826 - assign windows to Activities in task manager unriable
Summary: assign windows to Activities in task manager unriable
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Task Manager and Icons-Only Task Manager (show other bugs)
Version: 5.9.2
Platform: Neon Linux
: NOR normal
Target Milestone: 1.0
Assignee: Eike Hein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-23 00:20 UTC by Gauthier
Modified: 2017-07-25 21:55 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gauthier 2017-02-23 00:20:19 UTC
Hello,

Very glad to see that feature has been added, however it is not as reliable at as it is from the window menu. When trying move move windows to a different activity from task manager, it only works sometimes, whereas it always works from the window menu.

Most of the time if I use the activity manager to switch a window to a different activity (say I'm on activity 1 and I want to switch one window to activity 2), nothing happens and when I go back again to the activity menu from task manager nothing has changed (activity 2 is not ticked). Sometimes it does work though but I couldn't establish a pattern of when it does/doesn't work to provide more details.

This bug was present in Plasma 5.8.x and still present in 5.9.0. Sorry I should have reported it before.

Best

Gauthier
Comment 1 Eike Hein 2017-02-23 12:44:49 UTC
David?
Comment 2 David Edmundson 2017-02-24 04:01:01 UTC
Confirmed, checking any menu item isn't working.

menuItem.clicked.connect((function(activityId) 
   var checked = menuItem.checked;

is always false even though we clearly just checked the damn thing.
I have no idea where that puts the real bug. Debug in p-f shows the QAction is unchecked at the time of calling menuItem.checked

Virtual desktops doesn't have that problem because it always knows clicking == activate this index, activities are more complex because of the cardinality.
Comment 3 Eike Hein 2017-02-24 08:24:09 UTC
I didn't look into this at all yet, but FWIW from my experience in this code it's usually some scoping/closure related code where some captured variable has an outdated value or something isn't actually reachable and something else hides the ReferenceError.
Comment 4 Gauthier 2017-02-24 11:09:24 UTC
Should status be changed to confirmed?

(who is supposed to changed status btw? Is it the person who opened the ticket or the person who is actually trying to solve the problem...I wish my programming skills would be better btw so I could be helpful in actually solving issues I report!)
Comment 5 David Edmundson 2017-02-24 14:29:40 UTC
Git commit e5df3ded85c94f0a33afe12b18e6afad96f12639 by David Edmundson.
Committed on 24/02/2017 at 14:24.
Pushed by davidedmundson into branch 'Plasma/5.9'.

Avoid capturing MenuItem instead determine checked state from toggled signal

Summary:
Due to JS lambdas menuItem is being captured by reference, and as such
the call to menuItem.checked returns the checked state of the last item
in the for loop. Instead of adding to the capture this patch uses the
supplied boolean from the QMenuItem::toggled signal.

Also unlike Array.concat Array.splice alters the current array, we don't
want to create a new var.

Test Plan:
Debug of menuItem.text showed we were capturing the wrong thing
Clicked window into multiple activities and unticked them. All worked as intended

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D4757

M  +5    -6    applets/taskmanager/package/contents/ui/ContextMenu.qml

https://commits.kde.org/plasma-desktop/e5df3ded85c94f0a33afe12b18e6afad96f12639
Comment 6 Gauthier 2017-04-20 12:34:58 UTC
This now works well indeed, great work and thanks!

Just a small suggestion for usability...(not a bug as such):

Currently when you tick activity from the menu, the menu closes straight meaning that you got to reopen the menu if you want to tick or untick another activity. That means that you can do only one activity choice a the time or you have to open the menu several time. Again not a big deal but thought I'd flag it out.

Cheers

Gauthier
Comment 7 Gauthier 2017-07-06 13:33:01 UTC
Hello, could this fix be back ported to plasma 5.8 in the next bug fix release?
Comment 8 Eike Hein 2017-07-07 06:40:38 UTC
Sure.
Comment 9 Gauthier 2017-07-25 13:27:36 UTC
Thanks! Now just need to wait till next 5.8 release :)
Comment 10 Eike Hein 2017-07-25 21:55:01 UTC
Yep, I backported it on July 7th.