marking as RBS, please change if required

My change made scrollers TAB-focusable. What happens here is probably that focus goes to a scroller (an overflow div) and we don't see that scroller being focused because its focus ring is hidden by 'outline: none'.

Here it looks like we don't want the scroller to be TAB-focusable so we need to give it tabindex="-1".

Assigning to dpapad@ because I see you added overflow CSS rules to settings_ui.html.

@hugoh: I have to check whether tabindex -1 is an option anymore, since it might be affected by https://bugs.chromium.org/p/chromium/issues/detail?id=896624 (which is an intentional change).

@hugoh: The tabindex -1 works for settings, because the scrolling container is a <div>, but it does not work for History, which has also regressed as a result of this change.

In History's case, the scrolling container is a Custom Element, and because of  issue 896624  (which per Blink team is an intentional spec change), adding tabindex -1 at [1] prevents anything within that Custom Element to be in the tab order.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_history/app.html?l=92
See 

There is a similar bug in Print Preview's destinations dialog. Currently focus does not disappear since we do not have outline:none, but the behavior with the scroller focusable is somewhat odd. Tabbing into the list selects the entire list, and hitting down does not scroll it, but instead moves focus to the second list item (skipping the first one). Ideally the list should be removed from the tab order, but since it is an iron-list we run into the same problem as described in comment 4.

dpapad@, could you fix this bug (Settings' search box) by tabIndex=-1 and create a new bug about what you see in History?

> dpapad@, could you fix this bug (Settings' search box) by tabIndex=-1 and create a new bug about what you see in History?

Filed separate bug for History at https://bugs.chromium.org/p/chromium/issues/detail?id=906729.


CL that fixes Settings at https://chromium-review.googlesource.com/c/chromium/src/+/1343197.

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

commit 42c0440fcedf33a9eae9d8049ba2900d10bdb5be
Author: dpapad <dpapad@chromium.org>
Date: Tue Nov 20 01:27:55 2018

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}
[modify] https://crrev.com/42c0440fcedf33a9eae9d8049ba2900d10bdb5be/chrome/browser/resources/settings/settings_ui/settings_ui.html

Update: 
Rechecked the above issue on canary build #72.0.3617.0 using Windows(7,8,8.1,10) ,Mac(10.13.1 ,10.13.6 ,10.14.2) and Linux(14.04 LTS) and it is fixed.

Deleted: Fixed_Behaviour.mov 3.1 MB

	Fixed_Behaviour.mov 3.1 MB View Download

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

This regression should still be fixed currently since the Blink change that caused it has been rolled back, so removing RBS. Re-opening in order to track this as an issue that needs to be fixed before re-landing the Blink change.

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.

Deleted: Fixed_Result.mov 3.3 MB

	Fixed_Result.mov 3.3 MB View Download