New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 801162 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 907284
issue 585413
issue 891658



Sign in to add a comment

Spatnav can't focus scrollers

Reported by hu...@vewd.com, Jan 11 2018

Issue description

Reproduction steps:

1. out/Release/content_shell --enable-spatial-navigation third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-clipped-overflowed-content.html 
2. Go DOWN to the bottommost scrollable div D.
3. Scroll D to the RIGHT, using spatnav, until D displays none of its green boxes.
4. Leave D
5. Try to go back into D.

Expected: D should still be focusable (the user might want to scroll it again).
Actual: D is now impossible to focus using spatnav. 

 

Comment 1 by hu...@vewd.com, Jan 11 2018

Labels: spatial-navigation

Comment 2 Deleted

Comment 3 Deleted

Comment 4 by junho092...@lge.com, Jan 12 2018

Do you mean bottom most scrollable is horizontal overflow div?

I think reason for this behavior is that scrollable area is not focusable, right?

Comment 5 by hugo@chromium.org, Jan 12 2018

Yes.
Yes. :)

Comment 6 by hu...@vewd.com, Jan 14 2018

Blockedon: 585413

Comment 7 by hu...@vewd.com, Jan 15 2018

Description: Show this description

Comment 8 by hu...@vewd.com, Jan 15 2018

Manual test case:

<a href="#">a</a>
<div style="height: 120px; overflow: scroll;">
  spatnav should allow scrolling downwards<br><br><br>
  <br><br><br>spatnav should allow scrolling upwards
</div>

Solution in two steps:

1. Allow spatnav (and TAB-navigation) to focus scrollable areas i.e. treat scrollable areas as divs with tabindex=0 ( Issue 798094 ).
2. Allow spatnav to scroll a focused scrollable area*.

=> Use one extra key stroke to first focus the scrollable area. Once the area is focused, spatnav may* scroll it.

* If the area has any visible "focusables", spatnav will navigate to (=focus) one of those instead of scrolling.

Suggested algorithm: http://bit.ly/snav2 (currently under review).
Summary: Snav can't focus scrollers that has no focusables (was: Spatnav cannot focus scrollable divs that have no visible focusables)
Summary: Spatnav can't focus scrollers (was: Snav can't focus scrollers that has no focusables)
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 4

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

commit 5223c5c89cdef4918cd52d907148ab91a379f7bd
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Thu Oct 04 06:51:03 2018

Snav: Convert snav-only-clipped-overflow-content.html to snav-testharness.js

Let's also assert focus movements to the other focusables. Previously,
the only thing this test tested was that DOWN would _not_ move focus...

Once we've fixed  Issue 801162 , this test's expectations will change:
We want spatnav to first _focus_ the scroller (before scrolling it).

Bug: 803086,  801162 
Change-Id: I5c8a964c2900fb3a550a55ba9a975c7a33ef42dd
Reviewed-on: https://chromium-review.googlesource.com/c/1257832
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#596527}
[delete] https://crrev.com/2aba1b0d18473ad68ca3cecb81836253e33fe5d9/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-only-clipped-overflow-content-expected.txt
[modify] https://crrev.com/5223c5c89cdef4918cd52d907148ab91a379f7bd/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-only-clipped-overflow-content.html

Blockedon: 891658
Owner: hu...@vewd.com
Status: Started (was: Available)
Work-in-progress: https://chromium-review.googlesource.com/c/chromium/src/+/1308197.

With above CL, spatnav focuses a scroller before searching it. 

Blocked on  Issue 891658  because we need a reliable method to test focus movements to, from, and inside of iframes.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 2

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

commit a417a16cbf123394a09dc3ee985591da1b5301ed
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Fri Nov 02 17:01:10 2018

Snav: Convert all snav*iframe*.html tests to snav-testharness.js

The changes to snav-testharness.js lets us assert navigations
in and out of iframes:

 1) To be able to test focus movements in and out of iframes,
    we need to listen for 'keyup'-events on the document where
    focus is _about_to_go_.

    'keyup' targets the currently focused document. When 'keyup'
    comes, 'keydown' has already changed focus to the new document.

 2) Previously each movement had its own async_test(). Now we use
    one async_test() with many "test steps": one for each movement.
    This helps us avoid tricky (mis)usage of testharness.js's
    fetch_tests_from_window().

The changes to the HTML files are mostly mechanical except that
I gave some elements new or renamed ids so they become easier
to distinguish when debugging.

Once we've fixed  Issue 801162 , the tests' expectations will change:
We want spatnav to first _focus_ the iframe (before scrolling it
or searching inside of it).

Bug: 803086,  891658 ,  801162 
Change-Id: Ib53bb224398510b5744927a5d892381870fa4690
Reviewed-on: https://chromium-review.googlesource.com/c/1314469
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#604962}
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/resources/iframe.html
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/resources/snav-testharness.js
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-nested-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-nested.html
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-no-focusable-content-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-no-focusable-content.html
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-no-scrollable-content-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-no-scrollable-content.html
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-recursive-offset-parent-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-recursive-offset-parent.html
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-offscreen-focusable-element-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-offscreen-focusable-element.html
[delete] https://crrev.com/4dea3aba8069abf0688bef434173cc620ce87139/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element-expected.txt
[modify] https://crrev.com/a417a16cbf123394a09dc3ee985591da1b5301ed/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element.html

Blockedon: 907284
Cc: vollick@chromium.org bokan@chromium.org chaopeng@chromium.org mthiesse@chromium.org
Cc: hu...@vewd.com
 Issue 914831  has been merged into this issue.
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 10

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

commit 943ae69ddb9c9bf268fa03083643e9db923787ac
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Thu Jan 10 10:04:14 2019

Snav: Navigation in and out of iframes and overflow elements

Design doc: http://bit.ly/snav2

Thanks to "KeyboardFocusableScrollers" ( Issue 585413 ), which
we now ship to users of --enable-spatial-navigation, spatnav
can finally navigate to all kinds of scrollable elements.

How does the navigation look?
A lot like sequential navigation! Spatnav focuses scrolling
elements *before* searching them. Sequential navigation,
after  Issue 585413 , works in this way too, so that's nothing
controversial. Before this CL, spatnav could scroll an
unfocused (!) element. You could see this weird behavior at
snav-clipped-overflowed-content.html.

How could you remove so much code?
KeyboardFocusableScrollers and a simplified condition* for
iframes allows us to remove spatnav's backtracking logic.

* Now spatnav sees iframes as focusables; even if they
cannot be scrolled and even if they don't contain any other
focusables.

Bug:  585413 ,  801162 ,  914831 

Change-Id: I0f73fea7682f5dac48290a1142ae3817271cc628
Reviewed-on: https://chromium-review.googlesource.com/c/1308197
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Cr-Commit-Position: refs/heads/master@{#621526}
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/content/renderer/render_view_impl.cc
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/public/platform/web_runtime_features.h
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/renderer/core/page/focus_controller.cc
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/renderer/core/page/focus_controller.h
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/renderer/core/page/spatial_navigation.cc
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/renderer/core/page/spatial_navigation.h
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/renderer/platform/exported/web_runtime_features.cc
[delete] https://crrev.com/6e83997c8bacf3fcb37b751869abd4bacfc7c14f/third_party/blink/web_tests/fast/spatial-navigation/snav-backtrack-out-of-non-navigable-iframe.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-clipped-overflowed-content.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-div-scrollable-but-without-focusable-content.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-nested.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-no-focusable-content.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-no-scrollable-content.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-recursive-offset-parent.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-with-offscreen-focusable-element.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-only-clipped-overflow-content.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-search-beneath-exited-scroller.html
[modify] https://crrev.com/943ae69ddb9c9bf268fa03083643e9db923787ac/third_party/blink/web_tests/fast/spatial-navigation/snav-stay-in-overflow-div.html

Cc: -hu...@vewd.com
Owner: hu...@logikbyran.se
Status: Fixed (was: Started)
Fixed.

Tomorrow I leave Vewd to become a freelance consultant so changing owner to @logikbyran.se.

Sign in to add a comment