Focus rings not properly clipped by interior bounds of scroll view
Reported by
jshan...@etouch.net,
Nov 15 2016
|
||||||||||||||
Issue descriptionChrome Version:56.0.2920.0 (Official Build) 378fc3fd49ebb678fc98cd99831b858fd304691f-refs/heads/master@{#432057}-32/64 bit OS: Windows (7,8,8.1,10),Linux(Ubuntu 14.04 LTS) Precondition: Enabled Material Design in the rest of the browser's native UI flag from chrome://flags. Steps: 1. Launch Chrome, open NTP, right click on Bookmark bar and select 'Add folder' option 2. Click on 'New folder' button multiple time such that 12-14 folder is added and scroll bar appears. 3. Observe the focus ring of 'New folder' text box. Actual: Focus ring of 'New folder' text box overlaps the scroll bar in 'New folder' overlay. Expected: No such overlapping should be seen. This is a regression issue broken in M-55, will soon update the bisect info: Good Build: 55.0.2880.0 Bad Build: 55.0.2881.0 Note: Above issue is not seen on Mac OS.
,
Nov 16 2016
Narrow bisect: https://chromium.googlesource.com/chromium/src/+log/5379f1724fc3c193828e137faba13c8d59c3f228..1aceebcec9263879acc6610e69d9350b34380a31?pretty=fuller&n=100 Suspecting: r422921 ? Please help to re-assign if your change is not the cause for this issue.
,
Nov 16 2016
> Precondition: Enabled Material Design in the rest of the browser's native UI flag from chrome://flags. please stop marking bugs for features that are not enabled as p1 regressions.
,
Nov 16 2016
pkasting@ - would you please help fix this regression?
,
Nov 17 2016
It seems like the suspected CL in comment 2 is a good candidate for this, so maybe estade is the right owner. (So far I have yet to do any Harmony work, which bodes ill for my prospects of investigating this...)
,
Nov 17 2016
Indeed, I'm sure this is due to my code, but I'm no longer working on Harmony.
,
Nov 17 2016
I see :/ I guess I'll take the assignment, but who knows when I will ever get to Harmony work. Sigh.
,
Nov 17 2016
pkasting@ - there's nothing special about Harmony code. This is simply a bug in estade@'s Views code that needs to be fixed.
,
Nov 17 2016
Also, jschuh@ says that you are available to work on Harmony.
,
Nov 17 2016
I do intend to (someday) get to the Harmony work on my plate, but consider me unavailable for anything beyond this until further notice.
,
Jan 25 2017
Wondering if this is an issue on MacViews
,
Jan 25 2017
,
Apr 11 2017
Ananta, you want to take a look at this?
,
Apr 11 2017
,
Apr 11 2017
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7720633b71d3f5d8d3c912b93d2466a0b2972c24 commit 7720633b71d3f5d8d3c912b93d2466a0b2972c24 Author: ananta <ananta@chromium.org> Date: Wed Apr 12 03:48:16 2017 Fix an issue with the bookmarks bar in Harmony where in the focus ring would paint outside the scrollview. The proposed fix here is to scroll the parent scrollview by the row bounds to ensure that the ring does not paint outside the content area. There is an underlying problem here with the focus ring not getting clipped when it tries to paint outside the parent. The correct fix there is to notify the parents about the layer being used the focus ring which would require them to create layers to host the child layer. Clipping can be done then. The plan is to work on that in a subsequent patch. BUG= 665412 Review-Url: https://codereview.chromium.org/2813103002 Cr-Commit-Position: refs/heads/master@{#463923} [modify] https://crrev.com/7720633b71d3f5d8d3c912b93d2466a0b2972c24/ui/views/controls/tree/tree_view.cc
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c05d7143480feda606f6425f78f3b18598f9310 commit 5c05d7143480feda606f6425f78f3b18598f9310 Author: ananta <ananta@chromium.org> Date: Wed Apr 19 20:00:40 2017 Ensure that the focus ring in the bookmarks bar does not paint outside the parent view. The proposed fix is to notify the parent views when a child enables layering. The parent in this case the ScrollView overrides the newly added notification OnChildLayerChanged() and enables viewport layering. This ensures that the ring gets clipped. Longer term it seems like the focus ring should really be a property of the view and should not be instantiated by different controls all over the place. That for a later patchset. BUG= 665412 , 656198 TEST=Covered by test ViewObserverTest.ScrollViewChildAddLayerTest and ViewObserverTest.ChildViewLayerNotificationTest Review-Url: https://codereview.chromium.org/2813353002 Cr-Commit-Position: refs/heads/master@{#465720} [modify] https://crrev.com/5c05d7143480feda606f6425f78f3b18598f9310/ui/views/controls/scroll_view.cc [modify] https://crrev.com/5c05d7143480feda606f6425f78f3b18598f9310/ui/views/controls/scroll_view.h [modify] https://crrev.com/5c05d7143480feda606f6425f78f3b18598f9310/ui/views/view.cc [modify] https://crrev.com/5c05d7143480feda606f6425f78f3b18598f9310/ui/views/view.h [modify] https://crrev.com/5c05d7143480feda606f6425f78f3b18598f9310/ui/views/view_unittest.cc
,
Apr 21 2017
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by msrchandra@chromium.org
, Nov 15 2016Status: Untriaged (was: Unconfirmed)