Issue metadata
Sign in to add a comment
|
Regression: Focus on radio button is seen chopped from left side after pressing 'Tab' key on popup bubble.
Reported by
db...@etouch.net,
Mar 29 2017
|
||||||||||||||||||||||
Issue descriptionChrome Version: 58.0.3029.41 (Official Build)3d67f52b399689c5087b12bf6014dfb18fc11e42-refs/branch-heads/3029@{#465} 32/64-bit. OS: Windows(7,8,10) Pre-condition: Enable 'Material Design in the rest of the browser's native UI' flag What steps will reproduce the problem? 1. Launch chrome and navigate to http://www.popuptest.com/popuptest1.html 2. Click on blocked popup icon present in omnibox, press Tab key upto focus reaches to radio buton 3. observe focus on radio button. Actual: Focus on radio button is seen chopped from left side after pressing Tab key. Expected: Focus on radio button should not seen chopped from left side. This is regression issue, broken in 'M 57' and below is manual bisect: Good build:57.0.2977.0 Bad build:57.0.2978.0 Note: Issue is not seen on Mac Os.
,
Jun 22 2017
The issue here seems to be related to the RadioButton (and the Checkbox) being left-aligned according to the MD specs. The focus highlight is being clipped. The RadioButton control handles the painting of the focus ring itself instead of using the FocusRing class. IOW, it's painting directly onto it's own canvas which is clipped. This is probably an issue with the design of how the RadioButton and Checkbox are designed.
,
Jun 22 2017
I'm kinda surprised changing the alignment here changed this. I'd have expected this to get clipped by the button bounds either way, at least on Linux (I thought you could paint outside your own bounds on Win but not Linux). And in both cases it looks to me like we have plenty of room in the parent view, if that was the issue. It seems like if the focus ring goes around the radio circle itself, and not the whole button + text, that the radio button will have to be responsible for custom-drawing this. That may mean we need to reserve enough space for the full focus ring at all times. In turn that could either make these look as if they're "too far right", or else force us to position them "more left" to compensate. The spec suggests that focus rings on these as well as other elements are drawn outside the bounds of what the positioning rules are based on. I'm not sure if we have some magic way of doing this for things like buttons already. It might be easier to do in the "v2 spec" world where elements have their own inherent margins, because we can internally reduce the margin by the extra space needed here. I don't want to build something in the "v1" world to try and inset back from the spacing everywhere, that seems like a huge hassle. I'd rather ship with things looking like they're indented by 2 DIP. Maybe, though, there's some way of drawing the focus ring in some overlay or something that doesn't "count" as being part of the view bounds and isn't clipped by it? I'm not really sure.
,
Jun 23 2017
This is an issue with how the RadioButton and Checkbox handle their own focus-ring painting. An idea I have is to provide a delegation mechanism whereby the FocusRing class can optionally allow the focused control to paint. The RadioButton and Checkbox can then still do the painting of the focus ring, but it would paint on the FocusRing's canvas. This is how things work with most controls, for instance, look at the focus ring on the "Done" button in the left image. Using --draw-view-bounds-rects, you can see that the focus ring is another control laid over the button whose size provides space for the said ring.
,
Jun 26 2017
Using the FocusRing class and delegating the painting to the Checkbox and Radiobutton works. The focus ring is no longer clipped because it's painting on the FocusRing canvas is sized larger and is placed over the image.
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41f2ef33b224d02dea11bc5212811b39cc3b1115 commit 41f2ef33b224d02dea11bc5212811b39cc3b1115 Author: Allen Bauer <kylixrd@chromium.org> Date: Wed Jul 12 15:35:06 2017 Use a delegation to override FocusRing behavior for checkboxes and radio buttons. Bug: 706297 Bug: 683858 Change-Id: Ibb51dbacc26164ead556488da18fb322350fddff Reviewed-on: https://chromium-review.googlesource.com/546697 Commit-Queue: Allen Bauer <kylixrd@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#485963} [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/button/checkbox.cc [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/button/checkbox.h [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/button/radio_button.cc [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/button/radio_button.h [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/focus_ring.cc [modify] https://crrev.com/41f2ef33b224d02dea11bc5212811b39cc3b1115/ui/views/controls/focus_ring.h
,
Jul 12 2017
,
Jul 18 2017
Just to update: Above issue is fixed on latest dev build 61.0.3159.5 Thank you.
,
Jul 18 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rbasuvula@chromium.org
, Mar 29 2017Labels: hasbisect-per-revision OS-Linux
Owner: kylixrd@chromium.org
Status: Assigned (was: Unconfirmed)