Fixup of sequential focus navigation starting point doesn't work well in Shadow DOM |
||||||||||
Issue descriptionRepro steps, (also see attached HTML document): 1. Open a modal <dialog> that resides inside a Shadow root AND has a focusable child (like a button). 2) Close the dialog. 3) In the 'close' event handler, remove the dialog from the DOM. Expected: Focus should not be lost. Actual: Focus is reset to the start of the document. This is the root cause of issue 668313 . I am filing as a separate bug, since the former is a bit cluttered and has stagnated. Attempted myself to dig in the Blink code (which I am not familiar at all), and landed on [1] and [2] as potentially related. I would really appreciate it if someone from the Blink team can help. This is affecting our MD Settings page, and we need to make a decision as soon as possible on whether we should workaround the problem in our page, by not removing any <dialog> from the DOM (which could be non-trivial given the number of <dialog>s we have), or whether we can fix this inside Blink. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/ContainerNode.cpp?sq=package:chromium&dr&q=ContainerNode.cpp&l=518,552 [2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Node.cpp?l=2081
,
Apr 3 2017
,
Apr 3 2017
Just brainstorming (may be a terrible idea, but wanted to ask): > by not removing any <dialog> from the DOM Would it be reasonable to keep a single dialog in the DOM and re-purpose it? (Or a small number, if one dialog is not enough).
,
Apr 3 2017
@tbuckley: I don't think this bug should be marked as a blocker anymore. We are fixing the problem on our side at https://bugs.chromium.org/p/chromium/issues/detail?id=668313. @dschuyler: We would need to give up any notion of data binding if we were to do so. Our single dialog would need to be programmatically populated. I think this would be a much much larger change, that what is already implemented at issue 668313 .
,
Apr 3 2017
md-settings will just call .focus() a bunch to make up for the lack of <dialog> focus fixup behavior. we'd like for <dialog> to do this for us eventually, letting us remove our code. i think it's disappointing that nobody really owns <dialog> in blink (especially now that it's being implemented in Firefox) and we've gotten no responses on this issue. inb4: you're the new owner!
,
Apr 10 2017
,
Sep 29 2017
,
Sep 29 2017
,
Oct 1
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 12
<dialog> is unrelated to this issue. Sequential focus navigation starting point [1] doesn't work well if the focused element in Shadow DOM is disconnected. [1] https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation-starting-point
,
Oct 15
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b9a2fed10338d3be52b935414e6c87382ac7a75 commit 6b9a2fed10338d3be52b935414e6c87382ac7a75 Author: Kent Tamura <tkent@chromium.org> Date: Mon Oct 15 06:19:33 2018 Handle sequential focus navigation starting point in shadow trees correctly * Document::sequential_focus_navigation_starting_point_ count be a disconnected Range if a shadow tree including a focused element is disconnected. Introduce Range::FixupRemovedNodeAcrossShadowBoundary() to keep connected nodes in the Range. * Range::FirstNode() can be a ShadowRoot if a focused element in it is disconnected. We use FlatTreeTraversal instead of ElementTraversal to find a neighbor element. Bug: 705786 Change-Id: I51986133fd2ce01fe14a96d19c0ab4e477667feb Reviewed-on: https://chromium-review.googlesource.com/c/1278408 Commit-Queue: Kent Tamura <tkent@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#599575} [modify] https://crrev.com/6b9a2fed10338d3be52b935414e6c87382ac7a75/third_party/WebKit/LayoutTests/fast/events/sequential-focus-navigation-starting-point.html [modify] https://crrev.com/6b9a2fed10338d3be52b935414e6c87382ac7a75/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/6b9a2fed10338d3be52b935414e6c87382ac7a75/third_party/blink/renderer/core/dom/range.cc [modify] https://crrev.com/6b9a2fed10338d3be52b935414e6c87382ac7a75/third_party/blink/renderer/core/dom/range.h
,
Oct 15
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by dpa...@chromium.org
, Mar 29 2017