WebUI: cr.ui.FocusOutlineManager does not work with Shadow DOM. |
||||
Issue descriptionOutline manager currently defers updating the 'focus-outline-visible' CSS class until a focus event is received, which is problematic for Shadow DOM. Minimal repro example: https://jsfiddle.net/6v3rt0ae/1/. Minimal fix example: https://jsfiddle.net/4x70rzkc/1/. Proposed CL fix: https://chromium-review.googlesource.com/c/chromium/src/+/749887. I investigated git history to figure out why FocusOutlineManager delay updating the CSS class, and found the explanation at http://crrev.com/22521002/. Basically it is addressing the following case: 1) an element is focused by keyboard 2) while the element is focused the user clicks on it. Currently, the focus outline would remain visible, since no new focus event has fired yet. With the proposed fix, clicking on an element that already shows the outline will remove the outline. IMO that corner case is not important enough (and it is not obviously broken/fixed either way), compared to having a FocusOutlineManager that works/doesn't for Shadow DOM. Are there any objections on moving on with the proposed fix? cc'ing a few people who might have an opinion on this.
,
Nov 6 2017
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce67f79b1082968e5a0597572837cd6de6edcb9e commit ce67f79b1082968e5a0597572837cd6de6edcb9e Author: dpapad <dpapad@chromium.org> Date: Fri Nov 10 02:17:08 2017 MD Extensions: Fix FocusOutlineManager to work for Shadow DOM. FocusOutlineManager previously deferred updating the focus-outline-visible CSS class until a 'focus' event was received. This does not work in Shadow DOM, since 'focus' events do not reach the top level document element. Updating focus-outline-visible CSS class when a mouse or keyboard event occurs fixes the issue. Note: This slightly changes the behavior of FocusOutlineManager when 1) an element is focused by keyboard 2) while the element is focused the user clicks on it. Before, the focus outline would remain visible (introduced at crrev.com/22521002/). After, clicking on an element that already shows the outline will remove the outline. Bug: 772571 , 781459 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ie233c212fd17724ca8a43e46eb51398612644331 Reviewed-on: https://chromium-review.googlesource.com/749887 Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Commit-Position: refs/heads/master@{#515424} [modify] https://crrev.com/ce67f79b1082968e5a0597572837cd6de6edcb9e/chrome/browser/resources/md_extensions/compiled_resources2.gyp [modify] https://crrev.com/ce67f79b1082968e5a0597572837cd6de6edcb9e/chrome/browser/resources/md_extensions/error_page.html [modify] https://crrev.com/ce67f79b1082968e5a0597572837cd6de6edcb9e/chrome/browser/resources/md_extensions/error_page.js [modify] https://crrev.com/ce67f79b1082968e5a0597572837cd6de6edcb9e/ui/webui/resources/js/cr/ui/focus_outline_manager.js
,
Nov 10 2017
,
Nov 13 2017
Tested this issue on Windows 7 & Mac 10.12.6 as per C#0 & blocked bug-772571. 1.Tried with below fiddles still unable to reproduce the issue. Minimal repro example: https://jsfiddle.net/6v3rt0ae/1/. Minimal fix example: https://jsfiddle.net/4x70rzkc/1/. 2. Tried with proxy extensions but unable to reproduce the issue. dpapad@, Could you please provide us any sample extension/ clear repro steps to check this issue from TE end. Thanks in advance..!
,
Nov 13 2017
Tested on latest Canary-64.0.3267.0 & M63. |
||||
►
Sign in to add a comment |
||||
Comment 1 by dpa...@chromium.org
, Nov 3 2017Cc: scottchen@chromium.org