Adding known WebUI bugs. Some of these have been fixed (by adding tabindex -1).

Revert CL is at https://chromium-review.googlesource.com/c/chromium/src/+/1347053, should be landing soon.

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

commit 9a78e412ba0fd3bbae77d4b3fc7da31c0dd314af
Author: dpapad <dpapad@chromium.org>
Date: Wed Nov 21 21:23:11 2018

Disable Blink's KeyboardFocusableScrollers by default.

The original change at r608013 caused WebUI regressions, some of which are
unclear on how to fix because of a separate issue with tabindex -1 and Custom
Elements, https://github.com/w3c/webcomponents/issues/774.

Need to address WebUI affected cases, before the flag is re-enabled by defalut.

Bug: 907284
Change-Id: Ie8a41df1df663d25cf06a0ed6ae42ac80512df0c
Reviewed-on: https://chromium-review.googlesource.com/c/1347429
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610218}
[modify] https://crrev.com/9a78e412ba0fd3bbae77d4b3fc7da31c0dd314af/third_party/blink/renderer/platform/runtime_enabled_features.json5

@dapapad, will you take the action of re-enabling KeyboardFocusableScrollers once M72 has branched off? Next week?

(https://chromium-review.googlesource.com/c/chromium/src/+/1308197 depends on KeyboardFocusableScrollers.)

@hugoh: We have currently no way that I know of, for fixing the issue with shadowRoot scrollable containers as depicted at [1]. In [1], I also asked whether shadowRoots should be excluded from the silent insertion to the tab order, which I don't think it has been answered yet.

Until there is a viable way forward both for CustomElement containers and regular non-CustomElements contaniers, I don't see how we can pick this up again.


[1] https://github.com/w3c/webcomponents/issues/774#issuecomment-440905828

Did you see my suggestion in Issue 906729#11?

> Did you see my suggestion in Issue 906729#11?

Had not seen your suggestion, because I was not cc'ed on that bug. Responded there, https://bugs.chromium.org/p/chromium/issues/detail?id=906729#c12.

@dpapad, do you think you will have time to adapt WebUI before M73's branch?

So far, I think these WebUI regressions is the only thing that blocks us from re-enabling KeyboardFocusableScroller in M73.

> @dpapad, do you think you will have time to adapt WebUI before M73's branch?

Need to coordinate with the rest of the team to see whether we could tackle this for M73. I don't expect to have an update on this until early January when we do our Q1 planning, and other stakeholders are back from vacation.

@hugoh,

KeyboardFocusableScrollers initially caused two main issues for us: 1) it allows users to use the keyboard to focus on scrollable containers that we don't want them to focus on, and 2) when a user clicks on some text in the scrollable container, it moves focus to the container.

I understand that with "KeyboardFocusableScrollers try #2", you've fixed  issue 2 . But our workaround for issue 1 is to add a negative tabindex attribute, which forces these scrollable containers to now be mouse focusable, thereby re-causing  issue 2 . Is there any other workaround?

This is particularly a noticeable issue when a user clicks on some text in a scrollable container and then presses the tab key. The expected behavior is that the focus would go to whichever focusable element there is after where the user clicked on. But instead, when the user clicks, the focus goes to the scrollable container and therefore pressing the tab key focuses the first element in the scrollable container, often scrolling all the way to the top of the container.

Thoughts?

I see. Tricky... For ordinary HTML elements, perhaps something like this could work?

<div id="scroller" tabindex="-1" style="overflow: auto; width: 150px; height: 70px">
  <br><br><a href="https://www.google.se">link</a><br><br><br><br>
</div>

<script>
  let scroller = document.getElementById("scroller");
  scroller.onmousedown = function(e) {
    if (e.target == scroller) {
      // The scroller itself, not a child to it, is being clicked.
      e.preventDefault();  // Prevent focus movement.
      // preferredElement.focus().
    }
  }
</script>

However, if the scroller is custom element, perhaps you should ask the Polymer team to fix the elements you use? (KeyboardFocusableScrollers will affect other users of Polymer too.) I think they can fix this by setting delegatesFocus=true, see https://bugs.chromium.org/p/chromium/issues/detail?id=906729#c11.

@hugoh: Adding a mousedown/touchdown/pointerdown etc listener on all our scrollable containers does not sound like a scaleable workaround.

Moreover, regarding "delegatesFocus", even if we assume that the workaround for CustomElements at the comment you linked to works (needs to be verified), "delegatesFocus" only exists in Shadow DOM v1, and not Shadow DOM v0, which we are still using (we are in the process of migrating, https://crbug.com/738611).

IIUC, the problem happens on both Shadow DOM v0 and v1, when KeyboardFocusableScrollers is enabled, so that would not be sufficient either, at least for now.

Preventing default on mousedown events is also not ideal because it prevents some other default behavior, such as being able to drag to select text.