New issue
Advanced search Search tips

Issue 840236 link

Starred by 1 user
Status: Fixed
Owner:
tapted@chromium.org
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 840209
issue 828361



Sign in to add a comment

Views (all platforms) views::ScrollView's ScrollRectToVisibile() incorrectly accounts for RTL

Project Member Reported by tapted@chromium.org, May 7 2018 
Chrome Version       : 68.0.3422.0
OS Version: 10.0

What steps will reproduce the problem?
1. chrome://flags/#force-ui-direction set to RTL
2. Edit a bookmark and mash "New Folder" until there is horizontal scrolling
3. Scroll so that the deepest 'New Folder' is mostly out of the viewport
4. Click that deepest, 'New Folder' node.

What is the expected result?

Viewport should change so that the selected node is wholly visible. (see attached "ltr.png").

What happens instead of that?

Viewport scrolls elsewhere ("rtl.png")
ltr.png
33.7 KB View Download
rtl.png
25.4 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, May 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90ed378494841f712b909f0eff387687c551c343

commit 90ed378494841f712b909f0eff387687c551c343
Author: Trent Apted <tapted@chromium.org>
Date: Fri May 11 03:31:50 2018

RTL fixes for views::ScrollView

When scrolling with layers, flip the viewport with layer transforms
under RTL. This is necessary because the layer-backed scrolling
machinery is not privy to the automatic UI flipping done in
toolkit-views. ScrollOffsets must be positive, so scrolling
horizontally in RTL currently moves the contents to the left. Flipping
the viewport layer resolves this and scrolls to the right but reverses
the text, so flip the contents to compensate.

Testing exposed a quirk of View::ScrollRectToVisible() under RTL on
all platforms. It seems only Views::TreeView uses ScrollRectToVisible()
to affect horizontal scrolling, but it's currently broken (see
 http://crbug.com/840236 ). Fix it so we can use it in the test.

Bug:  840236 ,  828361 
Change-Id: Ib16724ff73829a6383be82e7bd11893b3a7e2ffc
Reviewed-on: https://chromium-review.googlesource.com/999076
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557778}
[modify] https://crrev.com/90ed378494841f712b909f0eff387687c551c343/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/90ed378494841f712b909f0eff387687c551c343/ui/views/controls/scroll_view_unittest.cc
[modify] https://crrev.com/90ed378494841f712b909f0eff387687c551c343/ui/views/controls/tree/tree_view.cc
[modify] https://crrev.com/90ed378494841f712b909f0eff387687c551c343/ui/views/view.cc

Comment 2 by tapted@chromium.org, May 11 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment