New issue
Advanced search Search tips

Issue 678595 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 307091



Sign in to add a comment

Change minimum height for overlay scrollbars to 32px

Project Member Reported by bokan@chromium.org, Jan 5 2017

Issue description

For Aura overlay scrollbars only.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 28 2017

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

commit 151a2db45f5fdbca8973988dd51d2bf6c86fbc8c
Author: bokan <bokan@chromium.org>
Date: Tue Mar 28 20:14:59 2017

Change minimum length of Aura overlay scrollbars.

It turns out Aura (and Android) overlay scrollbars weren't using the minimum
defined in NativeThemeAura. Instead, the minimum was set from
ScrollbarTheme::minimumThumbLength which just returns the thumb thickness.

After adding an override for ScrollbarThemeOverlay which correctly uses the
value form NativeThemeAura, the only consumers of
ScrollbarTheme::minimumThumbLength are the mock classes so I moved the base
implementation into the Mock class and made the base pure virtual.

BUG= 678595 

Review-Url: https://codereview.chromium.org/2734483002
Cr-Commit-Position: refs/heads/master@{#460199}

[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/content/child/webthemeengine_impl_android.cc
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.cpp
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.h
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.cpp
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.h
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.cpp
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.h
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlayMock.h
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/web/tests/FrameTestHelpers.h
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c/ui/native_theme/native_theme_aura.cc

Comment 2 by bokan@chromium.org, Mar 28 2017

Owner: bokan@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2017

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

commit d04d3a25c0615e015c3d8cb603ccab0081e350df
Author: bokan <bokan@chromium.org>
Date: Tue Mar 28 22:24:40 2017

Revert of Change minimum length of Aura overlay scrollbars. (patchset #9 id:160001 of https://codereview.chromium.org/2734483002/ )

Reason for revert:
Unit test failing on Android: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%20%28Nexus4%29/builds/62927

Original issue's description:
> Change minimum length of Aura overlay scrollbars.
>
> It turns out Aura (and Android) overlay scrollbars weren't using the minimum
> defined in NativeThemeAura. Instead, the minimum was set from
> ScrollbarTheme::minimumThumbLength which just returns the thumb thickness.
>
> After adding an override for ScrollbarThemeOverlay which correctly uses the
> value form NativeThemeAura, the only consumers of
> ScrollbarTheme::minimumThumbLength are the mock classes so I moved the base
> implementation into the Mock class and made the base pure virtual.
>
> BUG= 678595 
>
> Review-Url: https://codereview.chromium.org/2734483002
> Cr-Commit-Position: refs/heads/master@{#460199}
> Committed: https://chromium.googlesource.com/chromium/src/+/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c

TBR=estade@chromium.org,nasko@chromium.org,pdr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 678595 

Review-Url: https://codereview.chromium.org/2780923003
Cr-Commit-Position: refs/heads/master@{#460224}

[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/content/child/webthemeengine_impl_android.cc
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.cpp
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.h
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.cpp
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.h
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.cpp
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.h
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlayMock.h
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/web/tests/FrameTestHelpers.h
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/d04d3a25c0615e015c3d8cb603ccab0081e350df/ui/native_theme/native_theme_aura.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017

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

commit bca2d029eea47a55324d505a88329cd4112de03d
Author: bokan <bokan@chromium.org>
Date: Wed Mar 29 13:23:57 2017

Reland Change minimum length of Aura overlay scrollbars.

It turns out Aura (and Android) overlay scrollbars weren't using the minimum
defined in NativeThemeAura. Instead, the minimum was set from
ScrollbarTheme::minimumThumbLength which just returns the thumb thickness.

After adding an override for ScrollbarThemeOverlay which correctly uses the
value form NativeThemeAura, the only consumers of
ScrollbarTheme::minimumThumbLength are the mock classes so I moved the base
implementation into the Mock class and made the base pure virtual.

BUG= 678595 

Review-Url: https://codereview.chromium.org/2734483002
Cr-Original-Commit-Position: refs/heads/master@{#460199}
Committed: https://chromium.googlesource.com/chromium/src/+/151a2db45f5fdbca8973988dd51d2bf6c86fbc8c
Review-Url: https://codereview.chromium.org/2734483002
Cr-Commit-Position: refs/heads/master@{#460367}

[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/content/child/webthemeengine_impl_android.cc
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.cpp
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarTheme.h
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.cpp
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeMock.h
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.cpp
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.h
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlayMock.h
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/web/tests/FrameTestHelpers.h
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/bca2d029eea47a55324d505a88329cd4112de03d/ui/native_theme/native_theme_aura.cc

Labels: code-change
Status: Verified (was: Fixed)

Sign in to add a comment