New issue
Advanced search Search tips

Issue 914831 link

Starred by 1 user
Status: Duplicate
Merged: issue 801162
Owner: ----
Closed: Dec 13
Cc:
hu...@vewd.com
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Improved KeyboardFocusableScrollers

Project Member Reported by bokan@chromium.org, Dec 13 
Chrome Version: 72.0.3626.14
OS: All

What steps will reproduce the problem?
(0) Launch Chrome with --enable-spatial-navigation --enable-blink-features=KeyboardFocusableScrollers
(1) Visit https://bokand.github.io/spatnav-scroller.html?useSpatNav
(2) Move up and down on the page through the scrollable DIVs

There's two main issues I see here:

- A subscroller is scrolled before focus is moved onto it
- Cannot reverse scroll direction in a scroller without moving focus past it.

I've suggested some improvements in https://docs.google.com/document/d/1AjQdHSwF1kITTJeWIJPrAlSRYXWe9Zj1GEfV05ZZS1Q/edit#

Namely:
- Focus should move onto a scroller before it scrolls
- When focused, the SpatNav search should start from the edge of the scroller opposite to the direction of movement

Here's a JS mocked version of how that would feel (use Chrome without spatnav flags): https://bokand.github.io/spatnav-scroller.html

Comment 1 by bokan@chromium.org, Dec 13

Cc: hu...@vewd.com
Status: Available (was: Untriaged)
Hi Hugo,

I've played around a bit with keyboard scrollers and think there's a few things we could improve to make the experience better. WDYT?

Comment 2 by hu...@vewd.com, Dec 13 

Thanks for the feedback! This is something I've thought of too.

PTAL at https://chromium-review.googlesource.com/c/chromium/src/+/1308197, a CL I've had in-progress for a while.

Comment 3 by bokan@chromium.org, Dec 13

Mergedinto: 801162
Status: Duplicate (was: Available)
Hah, nice! That aligns exactly with what I was thinking. Thanks, I'll just dup this into the existing bug.
Project Member

Comment 4 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
►

Sign in to add a comment