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

Issue 802998 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Snav should keep its starting rect even though search scope is expanded.

Project Member Reported by junho092...@lge.com, Jan 17 2018

Issue description

What steps will reproduce the problem?
(1) ./chrome --enable-spatial-navigation
(2) Load attached file.
(3) Focus to top most element.
(4) Hit down key.
(4) Hit down key.

What is the expected result?
Focus should move to <a> element that has inner text as 'End'.

What happens instead?
Focus move to <a> element that has inner text as 'b'.

This behavior is introduced when snav failed to find focusable element in current search scope(container). Then, snav try to expand search scope to subtree of its parent container. And starting rect is changed by prev container's rect.
So we should keep focus element's starting rect in this case.


 
snav-iframe-with-outside-focusable-element.html
979 bytes View Download

Comment 1 by junho092...@lge.com, Jan 17 2018

Labels: -OS-iOS

Comment 2 by hu...@vewd.com, Jan 17 2018

I totally agree :) I'm looking forward to get this fixed.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 17 2018

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

commit 69520020e34d2c98ebcc342f852ccda11ebd2f74
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Wed Jan 17 14:01:09 2018

Snav: Keep the search start point constant throughout the search

Background:
If snav fails to find a focusable element in the first search
scope (activeElement's enclosing container C), snav expands
the scope to C's parent container.

starting_rect =
   visual rectangle from where snav looks for new focusables.

Focusables close to the starting_rect gets priority.

Problem:
As snav expands the scope, starting_rect changes from being
the enclosing container's visual rectangle to being the
visual rectangle of the parent container =>
This makes focus go to an undesirable element because snav's
use of a faulty "base" when evaluating distances to focusables.

Solution:
Keep the initial starting_rect throughout the search, even
if the search scope changes.

While here, add an early break that avoids an unnecessary layout.

TBR=szager@chromium.org, this was already discussed in:
https://chromium-review.googlesource.com/c/chromium/src/+/781326

Bug: 770147,  802998 
Change-Id: I00690a93d6a329ca30aa14432b9a7d25f3a3d65f
Reviewed-on: https://chromium-review.googlesource.com/870029
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Cr-Commit-Position: refs/heads/master@{#529730}
[add] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element-expected.txt
[add] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element.html
[modify] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/Source/core/page/FocusController.cpp

Comment 4 by hu...@vewd.com, Jan 17 2018

Status: Fixed (was: Started)

Sign in to add a comment