After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754959 - apps-menu items will not highlight
apps-menu items will not highlight
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2015-09-13 13:27 UTC by rudolf
Modified: 2015-10-15 20:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] Make apps-menu selection higlighting more reliable (844 bytes, patch)
2015-09-27 12:31 UTC, rudolf
rejected Details | Review
apps-menu: Fix up hover state after dropping grab (1.96 KB, patch)
2015-10-03 23:24 UTC, Florian Müllner
none Details | Review
apps-menu: Fix up hover state after dropping grab (1.36 KB, patch)
2015-10-04 00:10 UTC, Florian Müllner
committed Details | Review

Description rudolf 2015-09-13 13:27:04 UTC
Description of problem:

Submenu items will sometimes not highlight when pointing mouse cursor over them.

Problem exists in all gnome version I tried (3.8 - 3.17.91).


Steps to Reproduce:
1. click on Applications
2. move mouse to Favorites
3. move mouse to right, over first sub-menu item


Actual results:

Menu item is not highlighted in more than 50% of attempts, but clicking on item opens application correctly.


Expected results:

Highligting should be more reliable..
Comment 1 Matthias Clasen 2015-09-22 19:19:55 UTC
seems to be only the first time you enter the submenu, after the open submenu changed, and only for the item you enter initially. As soon as you veer over another item in the submenu, the selection is shown.
Comment 2 rudolf 2015-09-27 12:31:45 UTC
Created attachment 312238 [details] [review]
[PATCH] Make apps-menu selection higlighting more reliable

Hi, I think fixed this - could you please test my patch?

(disclaimer: I'm not programmer)

_onMotionEvent function has "if" statement where it queries current pointer grab of clutter. According to reference, it should return actor or NULL.
For some reason It does not work correctly if "if" statements uses negation operator (!) in condition.
 https://people.gnome.org/~gcampagna/docs/Clutter-1.0/Clutter.get_pointer_grab.html
Comment 3 rudolf 2015-10-03 07:03:00 UTC
I use patched version of apps-menu for almost week and it works perfectly.

Can you commit my patch?
Comment 4 Florian Müllner 2015-10-03 12:24:08 UTC
Review of attachment 312238 [details] [review]:

(In reply to rudolf from comment #3)
> I use patched version of apps-menu for almost week and it works perfectly.
> 
> Can you commit my patch?

No. Your patch does not work perfectly, it "fixes" the issue in this bug by breaking "triangle navigation" (meaning that when you move from the selected category towards a launcher icon, you don't accidentally switch the category on the way).

::: extensions/apps-menu/extension.js
@@ +178,2 @@
     _onMotionEvent: function(actor, event) {
+        if (Clutter.get_pointer_grab(null)) {

This doesn't make any sense - you are effectively turning this condition into "if (false)" (not to mention the added bogus parameter to a function with no arguments).
Comment 5 rudolf 2015-10-03 21:55:08 UTC
Thanks for clarification, it was stupid idea.
I played with code today and stripped _onMotionEvent and _isNavigatingSubmenu functions to the bone, and issue is still present:


 _isNavigatingSubmenu: function([x, y]) {
        let [posX, posY] = this.actor.get_transformed_position();

        if (posX > x || posX + this.actor.width < x)
            return false;

        if (posY <= y && posY + this.actor.height >= y)
            return true;

        return false;
    },

    _onMotionEvent: function(actor, event) {
        if (!Clutter.get_pointer_grab()) {
            Clutter.grab_pointer(this.actor);
        }
        this.actor.hover = true;

        if (this._isNavigatingSubmenu(event.get_coords()))
            return true;

        this.actor.hover = false;
        Clutter.ungrab_pointer();
        return false;
    },

If i comment out this line in CategoryMenuItem, problem disappears:

this.actor.connect('motion-event', Lang.bind(this, this._onMotionEvent));
Comment 6 Florian Müllner 2015-10-03 23:24:32 UTC
Created attachment 312614 [details] [review]
apps-menu: Fix up hover state after dropping grab

Category items grab the pointer to implement "triangle navigation", which
interferes with automatic hover tracking in other widgets. While this is
the correct behavior while we hold the grab (i.e. when crossing other
category items without switching), it can interfere with user expectation
when the grab is dropped, as the motion event that causes us to do so
doesn't necessarily occur before the "target"'s enter event - address this
by syncing up the hover state manually after dropping the grab.
Comment 7 Jasper St. Pierre (not reading bugmail) 2015-10-03 23:56:02 UTC
Review of attachment 312614 [details] [review]:

::: extensions/apps-menu/extension.js
@@ +193,3 @@
         Clutter.ungrab_pointer();
+
+        let pointerActor = global.stage.get_actor_at_pos(Clutter.PickMode.REACTIVE, x, y);

can this not simply be event.actor?
Comment 8 Florian Müllner 2015-10-04 00:10:37 UTC
Created attachment 312615 [details] [review]
apps-menu: Fix up hover state after dropping grab

Yup.
Comment 9 rudolf 2015-10-04 16:54:05 UTC
Last patch works fine on my systems
Comment 10 rudolf 2015-10-13 22:12:03 UTC
Just to remind, could somebody review the patch?
Comment 11 Florian Müllner 2015-10-15 20:36:25 UTC
Attachment 312615 [details] pushed as ca1da1b - apps-menu: Fix up hover state after dropping grab