New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 600036 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 593209



Sign in to add a comment

Better fix for overflow: auto scrolling

Project Member Reported by cbiesin...@chromium.org, Apr 1 2016

Issue description

https://codereview.chromium.org/1846023003/ is landing a fairly bad hack. The *right* solution is something like patchset 4 there, to just delay the clamping until after layout. However, that breaks a layout test. This bug tracks the proper fix for  bug 593209 

 
debugging notes...

(gdb) p box().hasOverrideLogicalContentHeight()
$6 = false
(gdb) p box().hasOverrideLogicalContentWidth() 
$7 = true
(gdb) p box().overrideLogicalContentWidth()
$8 = 100px
(gdb) p box().logicalWidth()
$9 = 100px
(gdb) p box().size().width()
$10 = 135px
(gdb) p box().layoutOverflowRect()
$11 = {
  m_location = LayoutPoint(0px, 0px), 
  m_size = LayoutSize(120px, 120px)
}

Width is 135px (it's the logical height, so grows with content + scrollbar), so it's 15px greater than the layout overflow rect, so we get an origin of -15px.

Cc: skobes@chromium.org szager@chromium.org
Starting to think we'll have to keep adjusting the scroll origin/offset as we lay out objects, but delay the clamping to the end.
If I adjust the scroll offset as the origin updates (so that the offset stays constant with respect to the origin), that actually fixes a bug in css3/flexbox/flexbox-overflow-auto.html

But it does break 14 other tests. So I'm still investigating if this is the right approach or not.

(btw, the bug in flexbox-overflow-auto is https://bugs.webkit.org/show_bug.cgi?id=76129 )
So... https://codereview.chromium.org/1867693002 *almost* works

The problem is RTL scrollbars. When an RTL scrollbar appears or disappears, the origin changes. BUT, the scroll position shouldn't change. For all other origin changes, the scroll position should change with them so as to keep the "real" position. (right...?)

Haven't yet found a solution for that.

Comment 6 by e...@chromium.org, Apr 25 2016

Cc: -szager@chromium.org cbiesin...@chromium.org
Owner: szager@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 1 2016

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

commit c9b3e99e2a71f7bcc4d9652916347782ff616e0d
Author: szager <szager@chromium.org>
Date: Wed Jun 01 12:33:47 2016

Include auto vertical scrollbar in intrinsicScrollbarLogicalWidth.

Prior to this change, auto vertical scrollbars could intrude into the
content box and overlap content on the right edge.  Discussion of
this change at:

https://codereview.chromium.org/1930183002/#msg12

BUG= 600036 
R=skobes@chromium.org,eae@chromium.org

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

[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/css3/flexbox/flexbox-height-with-overflow-auto.html
[add] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/scrollbars/auto-scrollbar-fit-content-expected.txt
[add] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/scrollbars/auto-scrollbar-fit-content.html
[add] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/scrollbars/auto-scrollbar-zero-width-block-expected.txt
[add] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/LayoutTests/scrollbars/auto-scrollbar-zero-width-block.html
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/c9b3e99e2a71f7bcc4d9652916347782ff616e0d/third_party/WebKit/Source/web/resources/listPicker.js

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 1 2016

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

commit b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Wed Jun 01 13:42:59 2016

Auto-rebaseline for r397112

https://chromium.googlesource.com/chromium/src/+/c9b3e99e2

BUG= 600036 
TBR=szager@chromium.org

Review URL: https://codereview.chromium.org/2028573004 .

Cr-Commit-Position: refs/heads/master@{#397125}

[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/android/fast/table/overflowHidden-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/android/scrollbars/scrollbars-on-positioned-content-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/android/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/android/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/fast/table/overflowHidden-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/linux/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/textarea/basic-textareas-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/textarea/basic-textareas-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/textarea/basic-textareas-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/textarea/basic-textareas-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.9/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/fast/forms/textarea/basic-textareas-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/fast/forms/textarea/basic-textareas-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/mac/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.txt
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.png
[modify] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/scrollbars/scrollbars-on-positioned-content-expected.png
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/scrollbars/scrollbars-on-positioned-content-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.png
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/virtual/prefer_compositing_to_lcd_text/scrollbars/scrollbars-on-positioned-content-expected.txt
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.png
[add] https://crrev.com/b5f0c236e0a4d677ff4cfb1a2bb310db94b27e90/third_party/WebKit/LayoutTests/platform/win7/virtual/rootlayerscrolls/scrollbars/scrollbars-on-positioned-content-expected.txt

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 4 2016

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

commit aff9f64394945f24400021fdec18dbd70eee3e14
Author: szager <szager@chromium.org>
Date: Sat Jun 04 02:05:05 2016

Refactor scroll updates during flexbox layout.

When there are nested flexboxes, the current code does some O(N^2)
work to update scrolling information after flexing has finished.  This
change streamlines the process for performing flex layout and updating
scrollbars into three distinct phases, controlled by the highest-level
flexbox in the layout tree:

1. Perform flex layout on descendants; any blocks with overflow:auto
scrollbars may add/remove scrollbars, but they will not run the normal
second-pass layout after changing scrollbars, and they will not clamp
their existing scroll positions.

2. If, during the first pass, any descendant added or removed
scrollbars, run a second flex layout pass, but don't allow any
descendants to add or remove scrollbars.

3. After the second pass, go through and clamp the scroll positions
on all scrolling descendants.

BUG= 593209 , 600036 

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

[add] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/LayoutTests/css3/flexbox/scrollbars-auto-expected.html
[add] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/LayoutTests/css3/flexbox/scrollbars-auto.html
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/layout/line/InlineBox.h
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/aff9f64394945f24400021fdec18dbd70eee3e14/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h

Status: Fixed (was: Started)
The patch in comment #9 implements the new behavior.  There were a couple of follow-on patches to fix issues with that patch:

https://codereview.chromium.org/1930183002
https://codereview.chromium.org/2088323003

Sign in to add a comment