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

Issue 593209 link

Starred by 4 users

Issue metadata

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

Blocking:
issue 600036



Sign in to add a comment

[REGRESSION] DevTools: navigator scroll gets reset

Project Member Reported by lushnikov@chromium.org, Mar 9 2016

Issue description

Version: 51.0.2672.0
OS: Linux

These are pretty complex reproduction steps; make sure to follow them
precisely.

What steps will reproduce the problem?
(1) Goto hoteltonight.com
(2) Open DevTools. Make sure the drawer is closed (if opened, close & reload devtools)
(3) Make sure devtools are docked to right. (if not, dock to right and reload devtools)
(4) Goto Sources panel, open navigator
(5) Make sure the navigator has a vertical scrollbar (resize the Browser window/expand folders if needed)
(6) Scroll navigator away from top.
(7) Click on any *.js file.

Actual: the scrollbar in navigator gets scrolled to top.


 
Owner: cbiesin...@chromium.org
Status: Assigned (was: Available)
Reverting https://crrev.com/1756003003 locally fixes the issue.
Cc: skobes@chromium.org szager@chromium.org e...@chromium.org
That sucks :(

I'll investigate tomorrow.
Labels: -Pri-2 Pri-1
Any updates on this? Despite a complex repro scenario, this happens a lot as you use devtools day-to-day.
Haven't had a chance to investigate yet, I downprioritized it because I thought you reverted it. I now see that the revert didn't land so I will get on this (and other regressions that may related)
Yeah, the revert didn't pass - the patch probably got covered by some other patch. Thank you for looking into this.
Sadly, my patch to fix all the other regressions (https://codereview.chromium.org/1811753003/) does not fix this issue. Looking more now.

Comment 9 by l...@chromium.org, Mar 24 2016

Cc: l...@chromium.org
Labels: ReleaseBlock-Stable
Any updates on this?
Working on it but this is very hard to track down. It's in progress.
Thank you, Christian! It would be great to not leak this into branch. Let me know if I can help you somehow.
Blocking: 600036
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 2 2016

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

commit ea799d37bd27d7ae346031a9332f542f7f57fd38
Author: cbiesinger <cbiesinger@chromium.org>
Date: Sat Apr 02 01:28:22 2016

Make sure to save/restore scroll positions in finishDelayUpdateScrollInfo

Due to Flexbox's multi-pass layout, we may lay out children at
intermediate sizes that are very small, causing us to clamp the
scroll position at this small size. Once we lay out at the right
size, the scroll position is lost.

To avoid this problem, make finishDelayUpdateScrollInfo store
the old scroll position so it can be restored.

XXXXX THIS IS TEMPORARY XXXXX

The plan is to bake this on canary and merge this to M50 to
fix the release blocker and fix this the right way on trunk
( bug 600036 ), by delaying clamping of scroll offsets to after layout.

BUG= 593209 

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

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

[modify] https://crrev.com/ea799d37bd27d7ae346031a9332f542f7f57fd38/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/ea799d37bd27d7ae346031a9332f542f7f57fd38/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/ea799d37bd27d7ae346031a9332f542f7f57fd38/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
[modify] https://crrev.com/ea799d37bd27d7ae346031a9332f542f7f57fd38/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Labels: Merge-Request-50 M-50
Status: Fixed (was: Assigned)

Comment 17 by tin...@google.com, Apr 3 2016

Labels: -Merge-Request-50 Merge-Approved-50 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M50 (branch: 2661)
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 3 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/638be1f77ce930f34389fc8040ccc1b014f91dfd

commit 638be1f77ce930f34389fc8040ccc1b014f91dfd
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Sun Apr 03 01:15:34 2016

Make sure to save/restore scroll positions in finishDelayUpdateScrollInfo

Due to Flexbox's multi-pass layout, we may lay out children at
intermediate sizes that are very small, causing us to clamp the
scroll position at this small size. Once we lay out at the right
size, the scroll position is lost.

To avoid this problem, make finishDelayUpdateScrollInfo store
the old scroll position so it can be restored.

XXXXX THIS IS TEMPORARY XXXXX

The plan is to bake this on canary and merge this to M50 to
fix the release blocker and fix this the right way on trunk
( bug 600036 ), by delaying clamping of scroll offsets to after layout.

BUG= 593209 

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

Cr-Commit-Position: refs/heads/master@{#384772}
(cherry picked from commit ea799d37bd27d7ae346031a9332f542f7f57fd38)

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

Cr-Commit-Position: refs/branch-heads/2661@{#467}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/638be1f77ce930f34389fc8040ccc1b014f91dfd/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/638be1f77ce930f34389fc8040ccc1b014f91dfd/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/638be1f77ce930f34389fc8040ccc1b014f91dfd/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
[modify] https://crrev.com/638be1f77ce930f34389fc8040ccc1b014f91dfd/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Cc: cbiesin...@chromium.org lushnikov@chromium.org
 Issue 600127  has been merged into this issue.
Labels: TE-Verified-50.0.2661.66 TE-Verified-M51
Verified the issue on Ubuntu 14.04 and its working fine on 50.0.2661.66.Please find the attached screen cast for the same.
593209_April_6.ogv
3.3 MB Download
Labels: -TE-Verified-M51 TE-Verified-M50
Still seeing this on 52.0.2729.3 dev-m (64-bit)


Google Chrome	52.0.2729.3 (Official Build) dev-m (64-bit)
Revision	30b869e0603513ecd7530f0215e6e79589f66b26-refs/branch-heads/2729@{#8}
OS	Windows 
Blink	537.36 (@30b869e0603513ecd7530f0215e6e79589f66b26)
JavaScript	V8 5.2.244
Flash	22.0.0.147
User Agent	Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2729.3 Safari/537.36
Command Line	"C:\Program Files (x86)\Google\Chrome\Application\chrome.exe" --disable-gpu --flag-switches-begin --enable-devtools-experiments --enable-fast-unload --enable-google-branded-context-menu --google-profile-info --enable-md-policy-page --enable-offline-auto-reload-visible-only --enable-offline-auto-reload --enable-tab-audio-muting --media-router=1 --show-saved-copy=primary --enable-smooth-scrolling --enable-features=DesktopSearchRedirection,ScrollAnchoring,StaleWhileRevalidate2 --flag-switches-end
Executable Path	C:\Program Files (x86)\Google\Chrome\Application\chrome.exe
Profile Path	C:\Users\Philip\AppData\Local\Google\Chrome\User Data\Default
Variations	d0bd833b-85fb2903
f2d919bf-85fb2903
16e0dd70-3f4a17df
31101bd6-f23d1dea
b3888d8d-ac4538a3
da89714-4ad60575
4a449931-f23d1dea
6345b824-3f4a17df
7c1bc906-f55a7974
cdd7eadf-f23d1dea
ba3f87da-a6404135
a5cb8590-3f4a17df
f049a919-3f4a17df
f1aba312-4ef7a373
31362330-3f4a17df
f15c1c09-ca7d8d80
dd4da2fc-3f4a17df
93731dca-3f4a17df
6d340565-ca7d8d80
165e16d1-3f4a17df
9e5c75f1-f0ee1b0f
2c3080ba-3f4a17df
64cbdfc2-ca7d8d80
f5dd6118-2f5721af
f79cb77b-3f4a17df
23a898eb-ca7d8d80
4ea303a6-6fb79ef8
4117e878-23622c9b
7aa46da5-b05adee3
9736de91-ca7d8d80
535ce4f1-6dd08681
dbffab5d-ca7d8d80
ad6d27cc-3e870323
ca314179-ca7d8d80
69bf80fa-3f4a17df
a35118-9597b6c7
867c4c68-3d47f4f4
12a73824-3d47f4f4
d747916f-f23d1dea
6844d8aa-669a04e0
3ac60855-486e2a9c
f296190c-18d1395b
4442aae2-4ad60575
ed1d377-e1cc0f14
75f0f0a0-6bdfffe7
e2b18481-bd104136
e7e71889-4ad60575
b39ea213-d1372334
46567c16-3f4a17df
8d27a1d0-3f4a17df
71140742-ca7d8d80
ed721c57-ca7d8d80
b0dc61a1-3f4a17df
Compiler	MSVC 2015
Hmm I can't reproduce in the same version. Could you provide a screencast? It may be better to file this as a new bug because it probably has a different root cause, as this one has been verified fixed -- a new bug would allow for easier finding of the cause.
Found it, looks like scroll anchoring breaks the devtools now. Should I file a bug for that?
Ah yes, please do!
Re #24: There have been recent changes to scroll anchoring that may have fixed this (such as r392774).  I'd suggest trying it on a canary build.
Project Member

Comment 27 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

Sign in to add a comment