[css-grid] Scroll reset position when updating inner html on content
Reported by
fi...@piktiv.se,
Apr 17 2018
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36 Steps to reproduce the problem: JS fiddle that reproduces the problem: https://jsfiddle.net/8ca1us4u/84/ What is the expected behavior? Scroll as usual. Works fine in Firefox/Edge What went wrong? Scroll position gets reset/jumps when updating inner html Did this work before? N/A Chrome version: 65.0.3325.181 Channel: stable OS Version: 10.0 Flash Version:
,
Apr 18 2018
I cannot reproduce this on Chrome 66.0.3359.117 on a Mac. I don't immediately have access to a windows machine right now to test.
,
Apr 19 2018
Able to reproduce the issue on reported chrome version 65.0.3325.181 using Windows 10 and Ubuntu 14.04. The issue is seen from M60(60.0.3112.0). Note: The issue is not seen on Mac 10.13.1 As the issue seems to be fixed on latest canary 68.0.3399.0, providing Reverse-Bisect info. Bisect Information: =================== Last Bad Build : 67.0.3377.0 First Good Build : 67.0.3378.0 While running tool bisect we have got all the bad builds, even after changing the range it didn't help. Hence providing with manual change log from omahaproxy. https://chromium.googlesource.com/chromium/src/+log/67.0.3377.0..67.0.3378.0?pretty=fuller&n=10000 As we are not very sure about the suspect from the above change log, requesting someone to help in assigning it to the right owner. Tentatively changing the status to Untriaged and adding component Blink>scroll, please change if this isn't apt. Thanks!
,
Apr 23 2018
sunyunjia@/bokan@: Could you please help us in identifying the CL from the above range which would have fixed this issue in M-67 and M-68. Thanks!
,
Apr 23 2018
I still see this in 68.0.3404.0. It only occurs for me on a low-DPI configuration (i.e. setting screen scaling to 100%). Given it's not a regression it's not Pri-1. Chao, please investigate when you can.
,
Apr 30 2018
Since this is a recent regression, please have a fix during M68 time frame.
,
Apr 30 2018
A cleaner example: https://jsfiddle.net/c0ddwymh/ This issue seems related to Scroll Anchoring. 1. For grid, we use a Layout to calc MinSizeForChild and this Layout calls ClampScrollOffsetAfterOverflowChange that reseted the scroll offset to 0,0. Example: 1) in chrome://flags, turn off #enable-scroll-anchoring 2) open https://jsfiddle.net/2b4ufc66/ scroll in red and click Stack trace see attachment. 2. Scroll Anchoring seems aid to fix the issue below. It find the anchoring by element. It does not update when user scroll less than current element height. When js change html, it first reset scroll to 0,0 then restore to previous anchor. These erase the user scroll. But if you scroll a huge amount it can stop at that point. I am not sure this is a bug. skobes WDYT?
,
Apr 30 2018
As #5, this is not a regression.
,
Aug 3
This is still an issue in 69.0.3497.23
,
Aug 3
The underlying bug here is that grid layout clamps prematurely. This happens even without scroll anchoring. Scroll anchoring makes some adjustments that alter the symptoms of this bug. But scroll anchoring isn't designed to compensate for incorrect clamping in other parts of layout. The correct fix is for LayoutGrid to avoid clamping. This can probably be done with DelayScrollOffsetClampScope, which is used by the flexbox code for a similar purpose. For the web developer, a workaround is to use out-of-flow positioning or "contain: strict" on the element being updated: https://jsfiddle.net/duak49rx/2/ This prevents the update from triggering relayout of the grid items. @eae, could you route this to someone on layout?
,
Aug 6
I'm on holidays next week but I can try to fix it this one. It seems the diff is as simple as just adding a line like skobes@ suggested: http://ix.io/1jjE/diff However I don't know how to write a test for this, any idea about that?
,
Aug 7
Re #11: Here is a test you can use.
,
Aug 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4b63701a1580e28962aa8693e767d74163b17aa commit c4b63701a1580e28962aa8693e767d74163b17aa Author: Manuel Rego Casasnovas <rego@igalia.com> Date: Wed Aug 08 00:10:38 2018 [css-grid] Fix scroll reset when updating content of grid item This patch simply uses DelayScrollOffsetClampScope to avoid resetting scroll position when the contents of a grid item are modified. BUG= 833837 TEST=fast/css-grid-layout/grid-item-scroll-position.html Change-Id: I74dd5e67d432b7b86b756ed741c7ad24243e6b8e Reviewed-on: https://chromium-review.googlesource.com/1165983 Commit-Queue: Manuel Rego <rego@igalia.com> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#581404} [add] https://crrev.com/c4b63701a1580e28962aa8693e767d74163b17aa/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-scroll-position.html [modify] https://crrev.com/c4b63701a1580e28962aa8693e767d74163b17aa/third_party/blink/renderer/core/layout/layout_grid.cc
,
Aug 8
Thank you very much skobes@! This should be fixed now. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by vamshi.kommuri@chromium.org
, Apr 18 2018