Regression: Shadow DOM v1 tabindex -1 behavior different than v0, causing WebUI regressions.
Reported by
sanyam.g...@etouch.net,
Oct 18
|
||||||||||||
Issue descriptionChrome version: 72.0.3584.0 (Official Build)Revision 945ab041ec6ac3fb389f94dcbfebb1839a12a69b-refs/branch-heads/3584@{#1}(32/64-bit) OS: Windows (7, 8, 8.1, 10), Mac(10.13.1, 10.13.6, 10.14) & Linux(14.04 LTS) What steps will reproduce the problem? 1. Launch chrome, navigate to chrome://settings/content/notifications 2. Now click anywhere on page and press tab key to traverse focus and observe. Actual Result : Focus does not traverse to ‘Iron’ icon after add button. Expected Result: Focus should traverse to ‘Iron’ icon. This is regression issue broken in ‘M-72’ and will soon update other info: Good build: 72.0.3583.0 (Revision: 600164) Bad build : 72.0.3584.0 (Revision: 600616)
,
Oct 18
,
Oct 18
,
Oct 23
I am able to reproduce this. No lead on what's happening yet.
,
Oct 23
This actually affects FocusRowBehavior and therefore regresses focus on various places of the UI, not just in chrome://settings/content/notifications. After some investigation with the help of scottchen@, we concluded that this is a difference between Shadow DOM v0 and v1. Minimal repro below. Shadow DOM v0: https://jsfiddle.net/2n0khjqc/2/, a child element is focusable even though it's parent has tabindex -1. Shadow DOM v1: https://jsfiddle.net/tbyp9hqg/2/, a child element is *not* focusable when the parent has tabindex -1. @tkent: Any thoughts on which behavior is the correct one? We would need to come up with some workaround, if Shadow DOM v1's behavior is expected.
,
Oct 23
,
Oct 23
,
Oct 24
as a hack, I tried passing delegatesFocus inside of Polymer (by munging Element.prototype.createShadow) here: https://jsfiddle.net/tbyp9hqg/3/ it made it such that *clicking* on the button allowed focus, but not keyboard i may have tickled this line: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/element.cc?sq=package:chromium&g=0&l=3442 but didn't have time to follow the turtles the whole way down (but i could be convinced to add some logs inside blink to track down further if needed)
,
Oct 24
,
Oct 24
IIUC, delegateFocus simply affects how :focus pseudo selector works. If delegateFocus is true, then :focus matches, even if a child is focused. In other words it basically makes :focus work similarly to how :focus-within works. But it does not affect "Tab" key behavior.
,
Oct 29
@tkent, @hayato: Could you take a look #5? We need to determine if we should come up with a work-around on our side, or if this needs to be fixed within Blink itself.
,
Oct 30
We don't have a good owner for focus navigation after kochi@ left. Anyway, let me take a look soon. @rakina, could you take over an ownership of focus related things from kochi@?
,
Oct 30
Confirmed: There *is* a difference between Shadow DOM v0 and v1, which is the expected behavior, as per https://w3c.github.io/webcomponents/spec/shadow/#sequential-focus-navigation Actually, there is a layout test for the current Shadow DOM v1' behavior; test5() in https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/shadow-dom/focus-navigation-with-delegatesFocus.html?q=focus-navigation-with&sq=package:chromium&g=0&l=106 So this is a intentional different behavior. We need a work-around on UI side, instead of Blink side.
,
Oct 31
@hayato: Can you explain a bit the difference between delegateFocus false/true (test6() in the link)? Would we be able to restore the previous functionality by using delegateFocus: true?
,
Nov 1
> @hayato: Can you explain a bit the difference between delegateFocus false/true (test6() in the link)? It looks there is no behavior change between in test5() and test6(), where tabindex=-1 is set. The whole shadow tree is skipped whether delegatesFocus is true or not if tabindex=-1.
,
Nov 9
,
Nov 15
Issue 901841 has been merged into this issue.
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13dfcd6d6ab09656870c8fba537230d82f7c21de commit 13dfcd6d6ab09656870c8fba537230d82f7c21de Author: rbpotter <rbpotter@chromium.org> Date: Thu Nov 15 22:22:35 2018 History and Settings: Remove tabIndex = -1 Setting tabIndex=-1 no longer allows child elements to be focusable in Shadow DOM v1 (see bug for additional context). Remove logic that does this, and in order to prevent focusing of the parent element when traversing backwards from the first item in the focus row, listen for shift+tab on this element. Bug: 896624 Change-Id: I35e871863418e7bca57f648713b8d172677a949c Reviewed-on: https://chromium-review.googlesource.com/c/1325352 Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Reviewed-by: Scott Chen <scottchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#608546} [modify] https://crrev.com/13dfcd6d6ab09656870c8fba537230d82f7c21de/chrome/browser/resources/md_history/history_item.js [modify] https://crrev.com/13dfcd6d6ab09656870c8fba537230d82f7c21de/chrome/browser/resources/settings/focus_row_behavior.js [modify] https://crrev.com/13dfcd6d6ab09656870c8fba537230d82f7c21de/chrome/browser/resources/settings/site_settings/site_entry.js
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/292d851b752f8a97a347ecdf8e1f412afe9bcc37 commit 292d851b752f8a97a347ecdf8e1f412afe9bcc37 Author: rbpotter <rbpotter@chromium.org> Date: Thu Nov 15 22:53:22 2018 Settings and History: Track when items are blurred When the tab index for the parent element is never set to -1, if focus leaves the list of items and then returns to it, the focus will automatically target the most recently focused control. Instead, if focus leaves the list, focus should return to the first control on the most recently focused item. Bug: 896624 Change-Id: I0e33e099f3b15b840d942d23fdccd620810b0806 Reviewed-on: https://chromium-review.googlesource.com/c/1336657 Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Reviewed-by: Scott Chen <scottchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#608562} [modify] https://crrev.com/292d851b752f8a97a347ecdf8e1f412afe9bcc37/chrome/browser/resources/md_history/history_item.js [modify] https://crrev.com/292d851b752f8a97a347ecdf8e1f412afe9bcc37/chrome/browser/resources/settings/focus_row_behavior.js
,
Nov 16
Update: Rechecked the above issue on Windows (7, 8, 8.1, 10), Mac(10.13.1, 10.13.6, 10.14.2) & Linux(14.04 LTS) OS using latest Canary build #72.0.3612.0 and the issue is fixed. Kindly refer the attached screen-cast for reference. Thank you..!
,
Nov 16
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by sanyam.g...@etouch.net
, Oct 18Owner: dpa...@chromium.org
Status: Assigned (was: Unconfirmed)