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 644134 - Dash keyboard navigation "jumps"
Dash keyboard navigation "jumps"
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-03-07 16:52 UTC by Florian Müllner
Modified: 2011-03-15 19:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
StContainer: fix focus navigation (2.02 KB, patch)
2011-03-07 19:19 UTC, Dan Winship
committed Details | Review
StContainer: Account for floating-point imprecision when sorting actors (2.14 KB, patch)
2011-03-07 20:34 UTC, Owen Taylor
committed Details | Review

Description Florian Müllner 2011-03-07 16:52:19 UTC
After moving key focus to the dash, the sequence of selected icons when hitting the down arrow is as follows: 1, 3, 4, 6, 8, ...
The up arrow behaves as expected.
Comment 1 Dan Winship 2011-03-07 19:19:04 UTC
Created attachment 182748 [details] [review]
StContainer: fix focus navigation

When navigating from a non-immediate descendant of a container, we
were attempting to use clutter_actor_get_transformed_position() to get
the exact position of that actor relative to the container.
Unfortunately, rounding errors caused this to sometimes decide that
widgets were overlapping, as opposed to stacked, causing it to skip
over some of them.

Instead, just use the position of the container's immediate descendant
that contains the deeper descendant; this is what we do for the
positions of the container's other children anyway, so it makes for a
more consistent comparison.
Comment 2 Owen Taylor 2011-03-07 20:33:49 UTC
Review of attachment 182748 [details] [review]:

If the problem is adjacent rectangles possibly overlapping - 1.000 vs. 0.999, seems that could as easily happen with the allocation box? Will attach my attempt at an alternative.
Comment 3 Owen Taylor 2011-03-07 20:34:06 UTC
Created attachment 182761 [details] [review]
StContainer: Account for floating-point imprecision when sorting actors

When we compare the boxes for two actors, they may appear to overlap
by a small amount because of floating-point imprecision. Allow for
up to 0.1 pixel overlap when determining what children are in the
focus direction from the currently focused actor.
Comment 4 Dan Winship 2011-03-07 22:25:50 UTC
Comment on attachment 182761 [details] [review]
StContainer: Account for floating-point imprecision when sorting actors

> If the problem is adjacent rectangles possibly overlapping - 1.000 vs. 0.999,
> seems that could as easily happen with the allocation box?

They're not *actually* overlapping, it's just that a tiny amount of error is apparently introduced by using get_transformed_position(). But yeah, the code probably shouldn't be depending on exact comparison of floating point anyway.

I think we want my patch as well anyway though. In any situation where the current code gave a different answer from the simplified version in my patch, you would also have a non-symmetric focus chain (ie, from A you could go Down to B, but from B you can't go Up to A.)
Comment 5 Owen Taylor 2011-03-08 00:04:54 UTC
(In reply to comment #4)
> (From update of attachment 182761 [details] [review])
> > If the problem is adjacent rectangles possibly overlapping - 1.000 vs. 0.999,
> > seems that could as easily happen with the allocation box?
> 
> They're not *actually* overlapping, it's just that a tiny amount of error is
> apparently introduced by using get_transformed_position(). But yeah, the code
> probably shouldn't be depending on exact comparison of floating point anyway.
> 
> I think we want my patch as well anyway though. In any situation where the
> current code gave a different answer from the simplified version in my patch,
> you would also have a non-symmetric focus chain (ie, from A you could go Down
> to B, but from B you can't go Up to A.)

You mean in a situation like:

   [ [ A ] ]
       [       [ B  ]        ]

? I guess that's true. Don't think we've ever had any reports of problems in GTK+, but the expectations of workingness for directional keynav have never been high. Seems sort of half-a-dozen-one-way half-a-dozen-the-other whether it's better to use the parent or not. I probably had some concrete:

 [ [ A ]                              ]
 [ [ B ] ]      [ [ C ]               ]

Example in mind when I wrote the GTK+ code, but don't think that's all that common.
Comment 6 Dan Winship 2011-03-08 01:27:24 UTC
(In reply to comment #5)
> Seems sort of half-a-dozen-one-way half-a-dozen-the-other whether
> it's better to use the parent or not.

sure, but the code is much simpler if you use the parent :)
Comment 7 Owen Taylor 2011-03-08 02:01:35 UTC
Review of attachment 182748 [details] [review]:

Fine with this, though if we're going to add my patch as well, probably good to rewrite the commit message a bit.
Comment 8 Owen Taylor 2011-03-15 19:45:11 UTC
Attachment 182761 [details] pushed as c0d0c79 - StContainer: Account for floating-point imprecision when sorting actors