Bug 332808

Summary: Kickoff starts application which is focused instead of clicked one.
Product: plasmashell Reporter: Bhushan Shah <bhush94>
Component: Application Launcher (Kickoff)Assignee: Sebastian Kügler <sebas>
Severity: normal CC: hein, lasse.liehu, leszek.lesner, mklapetek, notmart, sven.langkamp
Priority: NOR    
Version: master   
Target Milestone: 1.0   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Video showing the issue

Description Bhushan Shah 2014-03-30 13:37:40 UTC
1) Open applications menu
2) Select something (i.e internet)
3) Move mouse pointer from top.
4) click on last entry before it gets focus.
5) Notice that it starts wrong application, it should start one on which we cllicked.
Comment 1 Martin Klapetek 2014-04-02 13:17:40 UTC
Cannot reproduce here; can you provide a video/gif so we can check if we're trying to do the same thing?
Comment 2 Bhushan Shah 2014-04-02 14:37:22 UTC
Hard to catch in video, though I will try..
Comment 3 Marco Martin 2014-04-03 10:00:51 UTC
can't reproduce as well, no matter how fast i ham to click something else
Comment 4 Bhushan Shah 2014-04-07 11:35:23 UTC
Closing as I don't have proper steps to reproduce or can not capture the problem..
Comment 5 Bhushan Shah 2014-05-14 12:08:53 UTC
Comment 6 Leszek Lesner 2014-05-14 12:10:45 UTC
*** Bug 334760 has been marked as a duplicate of this bug. ***
Comment 7 Sebastian Kügler 2014-05-14 12:34:47 UTC
Hm, also here I cannot reproduce this issue. The focus moves instantly to the hovered item.

Maybe it's a problem with slow rendering?

Leszek: what hardware / driver are you using? Does moving the focus feel sluggish for you, or is it instant?
Comment 8 Leszek Lesner 2014-05-14 12:43:36 UTC
00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 4 Series Chipset Integrated Graphics Controller [8086:2a42] (rev 07) 
so its using the intel driver. 

The focus is moving instantly. 
Maybe it is another issue. As you can see in my video of the closed bug. I click on multimedia and before the animation starts drag the mouse to network to the top and it opens up then network though clicked on multimedia.
Comment 9 Leszek Lesner 2014-05-14 12:48:00 UTC
PS: It is reproducable in a virtual machine. At least I also tested it in virtualbox with the current neon5 build and it appears there also.
Comment 10 Sebastian Kügler 2014-05-14 13:02:08 UTC
Could you test it on real hardware? We know that virtual machines introduce a host of bugs due to their insufficient graphics stack.
Comment 11 Leszek Lesner 2014-05-14 13:05:13 UTC
I tested it on real hardware before trying to confirm this also with virtualbox. (The video is from a real hardware test)
Comment 12 Sebastian Kügler 2014-05-14 13:19:00 UTC
Can you tell more about the hardware, esp. the graphics bits? (Trying to get a feeling here of the circumstances under which this happens.)
Comment 13 Leszek Lesner 2014-05-14 13:43:33 UTC
It should be an Intel Graphics Media Accelerator (GMA) 4500MHD which comes with this Thinkpad x200s. (see the lspci output above) 
The compositor settings are set to:
Rendering backend: OpenGL 2.0 
Scale Method: Accurate 
VSync: Automatic 
Keep Window Thumbnails: Only for shown windows
The rest unchecked. 

Driver used: intel
Xorg: 1.15.1 
GLX Renderer: Mesa DRI Mobile Intel GM45 Express x86/MMX/SSE2 
GLX Version: 2.1 Mesa 10.1.0
Direct Rendering: Yes

CPU is Intel Core 2 Duo L9400 @ 1.8 Ghz
Hitachi hts72321 HDD 160 GB
Comment 14 Sven Langkamp 2014-05-14 21:09:19 UTC
I have the same problem with the neon5 iso on real hardware. If you move over the list in Kickoff it will select the correct entry on hover but once I have passed the top or bottom entry it will keep that selected and start the application no matter where the mouse is.
Comment 15 Sebastian Kügler 2014-05-16 00:42:55 UTC
Alright, got it. We need to reset the selected item once the mouse exits. That's an easy and a reasonable thing to do, and should cover all these cases.

Fix is following. Thanks a lot for helping to debug this!
Comment 16 Sebastian Kügler 2014-05-16 00:46:48 UTC
Git commit ad0c73b3ae0ee5d8a049c56e54cede13b815014d by Sebastian Kügler.
Committed on 16/05/2014 at 00:43.
Pushed by sebas into branch 'master'.

Reset currentItem in ListViews correctly

Without this patch, all items (apps, actions, etc.) would keep selection
state until someone else "steals" it. The problem is that in many cases,
it's not the mouse position that matters which item gets activated, but
the currently selected item in a given listview.

Adding an onExited handler to the onEntered handler fixes this, and thus
the bug that in some cases, the wrong app gets started.

M  +4    -0    applets/kickoff/package/contents/ui/KickoffItem.qml

Comment 17 Bhushan Shah 2014-05-18 05:00:34 UTC
*** Bug 334953 has been marked as a duplicate of this bug. ***
Comment 18 Lasse Liehu 2014-05-18 12:17:32 UTC
I can still reproduce this with plasma-desktop from today (cb80e47670298b874b580b722e0350958d52f90e).

Graphics card:
01:00.0 VGA compatible controller: NVIDIA Corporation G92 [GeForce GTS 250] (rev a2)

Compositor settings:
Animation Speed: slider position is in the middle
Rendering backend: OpenGL 2.0
Scale Method: Accurate
VSync: Automatic
Keep Window Thumbnails: Only for shown windows
OpenGL Platform Interface: GLX
Checkboxes below are unchecked.

Driver used: nvidia
Xorg: 1.15.1
Direct rendering: Yes
Comment 19 Sebastian Kügler 2014-05-19 09:20:58 UTC
Lasse: When leaving the menu, is there still an entry selected? Also, could you describe the issue you're seeing a bit more detailed?
Comment 20 Lasse Liehu 2014-05-19 09:55:43 UTC
Created attachment 86706 [details]
Video showing the issue
Comment 21 Sebastian Kügler 2014-05-19 10:02:09 UTC
Can you explain exactly what it's doing, and how it should be different? The video isn't really clear to me, no clear enough to get to the bottom of it.
Comment 22 Lasse Liehu 2014-05-19 10:04:54 UTC
Bug 334953 that was marked as duplicate has steps to reproduce and actual and expected results.
Comment 23 Martin Klapetek 2014-05-19 15:31:59 UTC
Yup, I can (still) confirm the original bug 334953.

Also this still happens: https://www.dropbox.com/s/c8k16amp4sheb0e/screencast-12.mkv

...for some reason the highlight is controlled "behind" the scollbar, which should not happen.
Comment 24 Sebastian Kügler 2014-05-19 23:18:16 UTC

Martin, this behavior is wanted here, in order to accommodate Fitt's law for the screen edges.
Comment 25 Sebastian Kügler 2014-05-20 00:05:03 UTC
Git commit f878cecf7d31a0e3164f17af32766684defdf073 by Sebastian Kügler.
Committed on 19/05/2014 at 23:59.
Pushed by sebas into branch 'master'.

read appview's currentItem before animating

moveRight() executed after the animation means we're querying for the
item not after it's clicked, but we have an item start the animation,
and once the animation is done, currentItem is checked, and opened.

Querying listview's currentItem before the animation fixes the wrongly
opened categories.

This should put the final nail in the coffin of

M  +7    -1    applets/kickoff/package/contents/ui/ApplicationsView.qml
M  +1    -2    applets/kickoff/package/contents/ui/KickoffItem.qml

Comment 26 Martin Klapetek 2014-05-20 13:11:43 UTC
> Martin, this behavior is wanted here, in order to accommodate Fitt's law for the screen edges.

That doesn't make sense, what screen edges usecase you have in mind? 

Controlling the view *outside* of the view's boundaries should not happen, that's not Fitt's law. It would be more sensible if you were controlling the scrollbar, then yes, but not selecting the items themselves (nowhere else that happens).

Plus it's offset, if you scroll down and move your mouse behind the scrollbar, the item at the top gets selected.
Comment 27 Eike Hein 2014-05-20 13:19:30 UTC
I haven't looked at the code, but from Martin's screencast and playing it with myself for a few seconds, I'm guessing that the code calls incrementIndex/decrementIndex  whenever an onEntered handler fires on $stuff (or the equivalent "we got a position event that intersects $rect"). That would explain why the current item deco can get out of sync with the cursor, since events aren't guaranteed to move between adjacent items (the cursor can leave the window), but increment/decrement always means a linear move. It's just a logic bug.
Comment 28 Eike Hein 2014-05-20 13:39:40 UTC
Addendum: The above is about the "plus it's offset" bug.

I do also agree that the mouse behavior is a bit wonky in general though - Fitts' Law is about pointing at things, and humans use their eyes in the process. I'm fairly sure that if you made a heatmap of where users actually click on Kickoff items you wouldn't see any clicks at the screen edge because the items aren't perceived to rest against it. The unexpected/surprising/defective-appearing behavior of events beyond the scrollbar registering (in contrast to any other scrollview we ship) weighs heavier.
Comment 29 Sebastian Kügler 2014-05-20 22:27:00 UTC
Git commit 30fe276c51cb025a4feaf35f65f31fe285937115 by Sebastian Kügler.
Committed on 20/05/2014 at 22:17.
Pushed by sebas into branch 'master'.

Simply Kickoff's mouse hover selection

This patch removes the MouseArea behind the listviews, which selected
the current item on hover beyond the listview's bounds. The positioning
in here isn't very reliable, especially when the content scrolls.

Simplify this by moving the margins inside the item and expanding the
listviews to the edges. This keeps the mouse interaction along the edges
intact, gives more expected behavior with the scrollbar, and doesn't
have the subtle positioning problems in finding the item.

This fixes wrongly selected entries in Kickoff when hovering over the
edges of the items, left and right, and fixes scrollbar behaviour.

M  +5    -4    applets/kickoff/package/contents/ui/ApplicationsView.qml
M  +1    -3    applets/kickoff/package/contents/ui/BaseView.qml
M  +1    -1    applets/kickoff/package/contents/ui/FavoritesView.qml
M  +2    -19   applets/kickoff/package/contents/ui/FullRepresentation.qml
A  +30   -0    applets/kickoff/package/contents/ui/KickoffHighlight.qml     [License: GPL (v2+)]
M  +1    -1    applets/kickoff/package/contents/ui/KickoffItem.qml
M  +1    -1    applets/kickoff/package/contents/ui/SearchView.qml
M  +1    -1    applets/kickoff/package/contents/ui/SectionDelegate.qml