Bug Fix: Scroll View Shows Unnecessary Scrollbar

Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize() explicitly in the Layout() implementation, horizontal scroll bar will always be visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463
Committed: https://crrev.com/8234584f65628f5f8c43341eaaa94dd0b1681f50
Cr-Commit-Position: refs/heads/master@{#417704}

[qiangchen, 2016-09-08 23:46:32]

Description was changed from

==========
Bug Fix: Scroll View Shows Unnecessary Scrollbar

BUG=644463
==========

to

==========
Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize()
explicitly in the Layout() implementation, horizontal scroll bar will always be
visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical
bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463
==========

[qiangchen, 2016-09-08 23:46:33]

qiangchen@chromium.org changed reviewers:
+ sky@chromium.org

[qiangchen, 2016-09-08 23:49:03]

A root fix for the previous ghost scrollbar issue.

I think my modified logic looks more reasonable.

PTAL. Thanks.

[sky, 2016-09-08 23:59:42]

Glad to see you got to the bottom of it!

https://codereview.chromium.org/2324043002/diff/1/ui/views/controls/scroll_vi...
File ui/views/controls/scroll_view.cc (right):

https://codereview.chromium.org/2324043002/diff/1/ui/views/controls/scroll_vi...
ui/views/controls/scroll_view.cc:296: void ScrollView::Layout() {
Please add test coverage.

[qiangchen, 2016-09-09 16:30:39]

The CQ bit was checked by qiangchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 16:31:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qiangchen, 2016-09-09 16:31:48]

Added Unittest. Thanks.

https://codereview.chromium.org/2324043002/diff/1/ui/views/controls/scroll_vi...
File ui/views/controls/scroll_view.cc (right):

https://codereview.chromium.org/2324043002/diff/1/ui/views/controls/scroll_vi...
ui/views/controls/scroll_view.cc:296: void ScrollView::Layout() {
On 2016/09/08 23:59:42, sky wrote:
> Please add test coverage.

Done.

[commit-bot: I haz the power, 2016-09-09 17:47:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 17:47:00]

Dry run: This issue passed the CQ dry run.

[sky, 2016-09-09 20:25:10]

Nice! Thanks, LGTM

[qiangchen, 2016-09-09 20:52:46]

The CQ bit was checked by qiangchen@chromium.org

[commit-bot: I haz the power, 2016-09-09 20:53:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 20:58:53]

Description was changed from

==========
Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize()
explicitly in the Layout() implementation, horizontal scroll bar will always be
visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical
bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463
==========

to

==========
Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize()
explicitly in the Layout() implementation, horizontal scroll bar will always be
visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical
bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463
==========

[commit-bot: I haz the power, 2016-09-09 20:58:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-09 21:01:32]

Description was changed from

==========
Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize()
explicitly in the Layout() implementation, horizontal scroll bar will always be
visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical
bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463
==========

to

==========
Bug Fix: ScrollView Shows Unnecessary Scrollbar

1. For bounded ScrollView, if its content view does not call SetSize()
explicitly in the Layout() implementation, horizontal scroll bar will always be
visible even if no overflow.

2. With |hide_horizontal_scrollbar_| set to true, we may find that the vertical
bar visible when the content height is equal to the container height.

This CL fixes the bugs.

BUG=644463

Committed: https://crrev.com/8234584f65628f5f8c43341eaaa94dd0b1681f50
Cr-Commit-Position: refs/heads/master@{#417704}
==========

[commit-bot: I haz the power, 2016-09-09 21:01:32]

Patchset 2 (id:??) landed as
https://crrev.com/8234584f65628f5f8c43341eaaa94dd0b1681f50
Cr-Commit-Position: refs/heads/master@{#417704}