Spatial Navigation: Up navigation no longer works after a page is scrolled
Reported by
innatere...@gmail.com,
Dec 20 2017
|
||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3298.3 Safari/537.36 Example URL: chrome://chrome-urls Steps to reproduce the problem: 1. Start Chrome with --enable-spatial-navigation. 2. Open chrome://chrome-urls. (for instance) 3. Hit End key. 4. Hit Up arrow key. What is the expected behavior? The lowermost focusable element is focused. In this instance, chrome://webrtc-logs should become focused. What went wrong? Page scrolls up. Does it occur on multiple sites: Yes Is it a problem with a plugin? No Did this work before? Yes The previous Dev release before 65.0.3294.5. That was the M64 branch point I think. Does this work in other browsers? Yes Chrome version: 65.0.3298.3 Channel: dev OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Up navigation now only works when the page has no top overflow.
,
Dec 21 2017
Able to reproduce the issue on reported chrome version 65.0.3298.3 and on the latest canary 65.0.3299.0 using Windows 10, Ubuntu 14.04 and Mac 10.13.1. Manual BIsect Range: ==================== Good Build: 65.0.3292.0 Bad Build: 65.0.3293.0 Change log: https://chromium.googlesource.com/chromium/src/+/5165b8fb056bfcfa6c92c12ebd9e7ad75729868e Review URL: https://chromium-review.googlesource.com/c/chromium/src/+/822159 Suspecting the same. Note: The above change log is from manual look up in omahaproxy, We are unable to provide the tool bisect as the invoked builds says "Aw, snap" every time for every search made. Adding label RB-stable feel free to remove if not required. @szager: Please confirm the issue and help in re-assigning if it is not related to your change. Thanks!
,
Dec 25 2017
,
Dec 29 2017
I tested this and I can confirm the issue. The old behavior felt more natural.
,
Jan 2 2018
Here's a fixup: https://chromium-review.googlesource.com/#/c/chromium/src/+/847513
,
Jan 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/260ccf1f403ad366a62fc2cd3a5a6831426cab06 commit 260ccf1f403ad366a62fc2cd3a5a6831426cab06 Author: Hugo Holgersson <hugoh@vewd.com> Date: Wed Jan 03 09:09:37 2018 Snav: Ensure spatnav uses the visual viewport as starting point When a page is opened, unless the page auto-focuses an element, the document <body> is the activeElement (aka focused element). In this case, spatnav uses heuristics in its search for the next focusable element to navigate to: the search starts at the visual viewport. This fixes the regression (caused by an adaption to root layer scrolling) described in the bug as well as adding 3 automated tests that check that spatnav correctly uses the visual viewport. Bug: 796493 Change-Id: Ie6cd71deaf1d7d7b100792feeaef57ab3fbff903 Reviewed-on: https://chromium-review.googlesource.com/847513 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: Hugo Holgersson <hugoh@vewd.com> Cr-Commit-Position: refs/heads/master@{#526655} [modify] https://crrev.com/260ccf1f403ad366a62fc2cd3a5a6831426cab06/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls [add] https://crrev.com/260ccf1f403ad366a62fc2cd3a5a6831426cab06/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-use-visual-viewport.html [modify] https://crrev.com/260ccf1f403ad366a62fc2cd3a5a6831426cab06/third_party/WebKit/Source/core/page/SpatialNavigation.cpp
,
Jan 3 2018
,
Jan 22 2018
I think #8 is another bug. Could you create a new Issue for it?
,
Jan 23 2018
I moved comment #8 and #10 to https://bugs.chromium.org/p/chromium/issues/detail?id=804669 . And removed that in this issue.
,
Jan 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/511906d0604cdd92df8aa077b82c51aa1a298525 commit 511906d0604cdd92df8aa077b82c51aa1a298525 Author: Hugo Holgersson <hugoh@vewd.com> Date: Tue Jan 23 15:54:32 2018 Snav: Always search for focusables in visual viewport first By initializing starting_rect to one of the visual viewport's edges we don't need to recalculate this value every time AdvanceFocusDirectionallyInContainer iterates. Q: So now we always use the frame's visual viewport when the focused element is off screen? A: Yes. The visual viewport is the default starting point for snav's search. This was already the implicit behavior users relied on (see bug). When the focused element F is off screen (say JS scrolled <body> downwards) we want spatnav to first look for already visible focusables, i.e not go to focusables nearby F because that will cause the window to scroll (bad UX). This lets users reach things that they can _see_ first. snav-ignore-offscreen-focused-imagemap.html (NEW) Bug: 770147, 796493 , 804242 Tests: snav-use-visual-viewport.html, Change-Id: Iaa861d3f75effe14eac558e2214dbc3af2df365b Reviewed-on: https://chromium-review.googlesource.com/873645 Commit-Queue: Hugo Holgersson <hugoh@vewd.com> Reviewed-by: Stefan Zager <szager@chromium.org> Cr-Commit-Position: refs/heads/master@{#531249} [add] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-ignore-offscreen-focused-imagemap.html [modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/FocusController.cpp [modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/SpatialNavigation.cpp [modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/SpatialNavigation.h |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by krajshree@chromium.org
, Dec 20 2017