+ Component: Blink>Focus

Additional information about testing environment:
You can enable spatial navigation by providing command line argument '--enable-spatial-navigation'
F.g: ./chrome --enable-spatial-navigation
Then, you can change focused element using 4-way directional arrow keys in keyboard.

Expected behavior in each test file:
1. container.html
There are two overflow areas. First focus to top element in top overflow area. Then push down key.
Focus should be moved to right-down element in same overflow area.
You can try same with below overflow area, it will work as expected.

2. offscreen.html
First focus to an element in overflow area. Then repeatedly push down key until focused element out of overflow area.
Then push down key once again. Intuitively scrolling should keep until cannot scroll anymore, but focus is moved to top element.

Able to reproduce the issue on 60.0.3112.113 using Ubuntu 14.04,Windows 7 with below steps

1.Launched chrome with --enable-spatial-navigation and navigated to container.html
2.Focused First element in overflow area >> Used Down arrow key and observed focus on button which is outside overflow area.
3.Navigated to offscreen.html >> Focused element in overflow area,Hit down arrow key untill focus is outside overflow area,Now focus is on first button outside
4.Now used down arrow key-- focus is on second button which is out of overflow area.

Same behaviour is seen in M50[50.0.2661.66] as well. Hence marking as Untriaged and adding appropriate labels for further inputs.

For the easy understanding, I attached a video file that shows the couple of problems.

As I see, those are definite problems and the expected behaviors are so obvious without a doubt.
If it seems clear problems to others as well, the issue reporter, my co-worker, will be able to work to fix it.
For one thing, we're working in LG electronics and considering to use more enhanced spatial navigation for LG webOS TV.

Deleted: snav_bugs.mp4 2.7 MB

	snav_bugs.mp4 2.7 MB View Download

Thanks for the bug reporting.  It looks like the problems are there.

But I'm not familiar with spatial navigation, cc'ing Hugo.

Hi, I uploaded a cl for fixing container problem:
https://chromium-review.googlesource.com/c/chromium/src/+/668315

I believe this will be a good start point to fix above problems.

Thanks for starting to fix the issue!
(maybe you cannot be in "Owner" field, if you haven't got committer status...)

I'm not familiar with the algorithms how spatial navigation works, so
I hope Hugo can help here.

FYI: I found another issue and created on: https://bugs.chromium.org/p/chromium/issues/detail?id=784320

I'd like to be able to group all spatnav-issues into its own bug category, say Blink>HTML>Focus>SpatialNavigation. kochi@, do you think that's a good idea? That might help triage and to get a better picture of spatnav's status.

Re c#10 if the number goes high, it makes sense to have another sub
component, but I'd like to avoid going too granular.

How about using a label as "spatial-navigation"?

I agree with kochi@.

Also It would be great if I can label an issue that I created.

JunHo, can't you add a label yourself?
Maybe it's "EditBugs" bit, I'll ask it for you.

Kochi, No I can't.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad615d334e3718645bcd4491f40d3ad684e05fad

commit ad615d334e3718645bcd4491f40d3ad684e05fad
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Mon Dec 18 18:24:45 2017

Snav: Start candidate search in the focused element's container

Problem:
Imagine the focused element F "scrolled out" = hidden outside
of its container C's viewable area. In this case spatnav would
start its search for focus candidates up at the document root
and therefor miss (possibly nearby) candidates in C.

Solution:
Let AdvanceFocusDirectionallyInContainer() use the currently
focused element F's container C as its starting rectangle unless
F is an HTMLAreaElement. For HTMLAreaElement the search starts in
the HTMLAreaElement's image.

Bug:  763206 
Change-Id: I4b721e92bf47ff5361d6f0fa8b70805eb0b485c2
Reviewed-on: https://chromium-review.googlesource.com/826345
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524743}
[modify] https://crrev.com/ad615d334e3718645bcd4491f40d3ad684e05fad/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-clipped-overflowed-content-expected.txt
[modify] https://crrev.com/ad615d334e3718645bcd4491f40d3ad684e05fad/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-clipped-overflowed-content.html
[modify] https://crrev.com/ad615d334e3718645bcd4491f40d3ad684e05fad/third_party/WebKit/Source/core/page/FocusController.cpp

The problem (2) in the bug description sounds like  crbug.com/770103 ? If yes, now that the first problem (1) is fixed, can we close this bug and focus on (2) in  crbug.com/770103 ?

hugo@, no, comment#19 fix problem (2). And I think problem (2) is not identical with   crbug.com/770103  . So this need keep open until problem (1) fixed.

Ok! I'm trying container.html and I can't see where it goes wrong. It doesn't work if I revert #19... To me it seems that #19 fixed both container.html and offscreen.html.

hugo@, would you please see a video in commet #3(1. inconsistent focus behavior)? The video shows clearly what problem is.
Or do you think the behavior is not problem?

junho0924.seo@ one question: if the top container is made scrollable, does the jump occur, as shown in (1)?

(2) seems to be a problem of snav logic starting over again (from the first focusable node in the DOM chain) when its currently focused node is offscreen(?).

Hi tonikitoo@, 

1. Do you mean jump is focus move? If right, yes. Because current snav recognizes scrollable overflow div as container. And it will starts search in the container first. So right-bottom side element in container will get focus.

2. Yes. And (2) is fixed by comment#19. So (2) should never occurs after comment#19 landed.

Re #23. Here the user could use the RIGHT key to reach the top div's second element. (If it was completely unreachable, it would have been a bigger problem.) But I get your point: We need to spec this.

The div is not scrollable (because it has no overflow content), still it has a scrollbar because of "overflow: scroll". 
Should such non-scrollable "overflow: scroll"-containers' focusables have priority anyway? Yes, why not... We could then spec:

  A) If the currently focused element resides in an "overflow: scroll"-container, that container's (visible) focusables have priority.

That's inline with my thinking in http://bit.ly/snav2 anyway :)

Alternatively we could per our current logic say: 

  B) If the currently focused element resides in a "overflow: scroll"-container _that is scrollable in the given direction_, that container's (visible) focusables have priority.

Indeed, B) is more complicated so doing A) might allow us to simplify the implementation a bit.

I believe that one of spatnav's functional requirements should be: "spatnav should be able to reach the same set of elements that TAB-navigation can reach".

Firefox's TAB-navigation doesn't focus *non-scrollable* "overflow: scroll"-containers. Only actual *scrollable* areas gets TAB focus. Chrome might, in the future, mimic Firefox's behavior, see  Issue 585413 . 

I think spatnav does the right thing already. It looks at actual scrollability (not only the overflow-property) => 
Here container.html's first div isn't scrollable => Its focusables should not get priority.