Regression: Focus navigation issue is observed on chrome://settings page.
Reported by
dchau...@etouch.net,
Nov 27
|
||||||
Issue descriptionChrome Version: 72.0.3623.0 (Official Build) Revision bc6ec85849f003e7b403d5e48eee959e76de0cd3-refs/branch-heads/3623@{#1} (32/64-bit) OS: Windows (7,8,8.1,10), Linux (14.04 LTS), Mac OS X(10.14.2,10.13.1,10.13.6) What steps will reproduce the problem? 1. Launch Chrome and navigate to chrome://settings page. 2. Scroll down the page and click on 'Advanced' button. 3. Click on blank area (in between Language and down arrow icon) of 'Language' section to expand it. (Refer video) 4. Now press 'Tab' key from keyboard and observe the focus. Actual: Unnecessary focus moves to 'Turn on sync' button under 'People' section on pressing 'Tab' key. Expected: Focus should move to up arrow icon of 'Language' section on pressing 'Tab' key. This is a regression issue, broken in M-72 series, below is manual regression range: Good build: 72.0.3610.0 (Revision: 607839) Bad build: 72.0.3611.0 (Revision: 608211) Kindly review the attached screen-cast for reference. Thank you..!
,
Nov 27
marking as RBS, please change if required
,
Nov 28
My feature, KeyboardFocusableScrollers, had already been disabled in 72.0.3623.0.
,
Nov 28
I redid the bisect with --disable-blink-features=KeyboardFocusableScrollers. Bisect range is: https://chromium.googlesource.com/chromium/src/+log/fc50c1477673ff62ae6243ab30de31fd4b430c01..42c0440fcedf33a9eae9d8049ba2900d10bdb5be so this is likely due to https://chromium-review.googlesource.com/c/chromium/src/+/1343197, which was an attempt to fix a different settings page regression due to the focusable scrollers. This bug seems similar to the Print Preview bug that surfaced due to a similar fix attempt, see https://crbug.com/907378. In that case I reverted the CL, since we decided to turn KeyboardFocusableScrollers off for now. We should probably do the same here.
,
Nov 28
@rbpotter: Thanks for the investigation. I agree that we should revert that CL for now as well. @hugoh: IIUC, the tab focusable vs click focusable issue is what you are referring to at https://bugs.chromium.org/p/chromium/issues/detail?id=908809#c3 ?
,
Nov 28
@dpapad, what I mean in Issue 908809 #c3 is that I will probably need to land a rewrite of KeyboardFocusableScrollers. Current KeyboardFocusableScrollers unintentionally affected click-focusing ( Issue 908809 ). Perhaps that's the root cause to some of the WebUI issues (Issue 907284) we've seen.
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/760f1f815c71b362e60a40ee421c10a6372e040b commit 760f1f815c71b362e60a40ee421c10a6372e040b Author: Rebekah Potter <rbpotter@chromium.org> Date: Wed Nov 28 22:39:45 2018 Revert "Settings WebUI: Add tabindex -1 in scrolling container to prevent tabstop." This reverts commit 42c0440fcedf33a9eae9d8049ba2900d10bdb5be. Reason for revert: This causes https://crbug.com/908775, and the Blink changes that were the reason for the change have been rolled back for now. Original change's description: > Settings WebUI: Add tabindex -1 in scrolling container to prevent tabstop. > > This is necessary because of recent Blink changes at r608013. > > Bug: 905999 > Change-Id: I111e3631e4d832afb48a4161552fc27a1bbb9630 > Reviewed-on: https://chromium-review.googlesource.com/c/1343197 > Reviewed-by: Rebekah Potter <rbpotter@chromium.org> > Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> > Cr-Commit-Position: refs/heads/master@{#609546} TBR=dpapad@chromium.org,rbpotter@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 905999, 908775 Change-Id: Ie14d8cbea36dcb91d03e8e7ab7b2fdd5010fcc10 Reviewed-on: https://chromium-review.googlesource.com/c/1354235 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/master@{#611906} [modify] https://crrev.com/760f1f815c71b362e60a40ee421c10a6372e040b/chrome/browser/resources/settings/settings_ui/settings_ui.html
,
Nov 29
Update: Rechecked this issue on Windows(7,8,8.1,10), Mac(10.13.1, 10.13.6, 10.14.2) and Linux(14.04) machines using latest Canary #72.0.3625.0 and issue is fixed. Hence adding TE-Verified labels. please refer the attached screen-cast for reference. Thank you.
,
Dec 3
Removing RBS, but keeping this open to track fixes needed before relanding the original Blink change.
,
Jan 8
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by dchau...@etouch.net
, Nov 27Owner: hu...@vewd.com
Status: Assigned (was: Unconfirmed)