top nav judder on techcrunch with scroll anchoring |
|||||||
Issue description1. On a phone, go to http://techcrunch.com/2016/04/05/will-an-apple-a-day-keep-webrtc-away/. 2. Scroll down until you reach the transition point where the top nav hides. 3. Notice the juddering page/top nav If keep scrolling, this issue is gone.
,
Apr 11 2016
,
Apr 13 2016
After some investigation, Minimized repro: https://jsbin.com/yabepiz It has something to do with the padding-top changing on body when the nav bar is hidden. Still need to investigate as to why this is happening.
,
Apr 13 2016
After more investigation, this seems like one of the fundamental problems of scroll anchoring. It can be reproduced without the nav bar and has to do with js reacting to the adjustment that we make. Minimized repro: https://jsbin.com/fenugo The minimized repro above calls a function every 20 ms that changes the padding-top of the body element from 100px to 50px if the scroll position is > 300 (and vice-versa if scroll position < 300). So this is what happens: 1. The user scrolls just past the 300px point 2. The js on the page changes the padding-top on the body from 100px to 50px. 3. ScrollAnchor sees this change, and adjusts the scroll offset by 50px upwards. 4. The js on the page sees that the scroll offset < 300px and changes the padding back to 100px. 5. ScrollAnchor sees this change, and adjusts the scroll offset by 50px downards. 6. The js on the page sees that the scroll offset > 300px and changes the padding to 50px 7. repeat from step 3 I guess the problem in general is that if the site itself is making adjustments, then our adjustments may cause undefined behavior.
,
Apr 13 2016
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2fd28b5102e73c49b3891d9bc7c05ffda979e2ea commit 2fd28b5102e73c49b3891d9bc7c05ffda979e2ea Author: skobes <skobes@chromium.org> Date: Wed May 18 21:27:18 2016 Implement bounce suppression in ScrollAnchor. An element can "bounce", by making the inverse of its last movement, when a scroll anchoring adjustment interacts with a scroll event handler that changes layout at a scroll offset threshold. ScrollAnchor now detects bounces by tracking the most recent adjustment and the element that triggered it (even if we have anchored to a different element since then), and suppresses repeat bounces. BUG= 598233 , 601906 , 603376 Review-Url: https://codereview.chromium.org/1971423002 Cr-Commit-Position: refs/heads/master@{#394552} [add] https://crrev.com/2fd28b5102e73c49b3891d9bc7c05ffda979e2ea/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/onscroll-bouncing.html [modify] https://crrev.com/2fd28b5102e73c49b3891d9bc7c05ffda979e2ea/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/2fd28b5102e73c49b3891d9bc7c05ffda979e2ea/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp [modify] https://crrev.com/2fd28b5102e73c49b3891d9bc7c05ffda979e2ea/third_party/WebKit/Source/core/layout/ScrollAnchor.h
,
May 24 2016
@skobes, the top nav judder issue still exists on TC with the debouncing fix above. I didn't get a chance to dig into it further.
,
May 25 2016
Exciting - the original Techcrunch page has:
@media (max-width: 29.74em) body {
transition: padding-top .2s ease-in-out;
}
The intermediate states of the animation defeat the bounce suppression. :(
,
May 25 2016
Lol. We should go back to the drawing board on how to address this then. :)
,
Jun 28 2016
I don't actually see this on that techcrunch page. I see a brief flicker, but the bounce suppression seems to be working.
,
Jun 28 2016
,
Jun 28 2016
This still repros for me with mobile emulation.
,
Jun 28 2016
I wish we had a more generic solution to the JS cycle problem ( issue 604996 has more discussion of it). Right now the best option I know of is to set a fixed limit (say 50) on the number of adjustments made between user scrolls.
,
Jun 30 2016
FYI, here is what it looks like with a limit of 20 adjustments between user scrolls.
,
Jul 1 2016
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34ae0389f0c660c23f691595341188acc5444e00 commit 34ae0389f0c660c23f691595341188acc5444e00 Author: skobes <skobes@chromium.org> Date: Fri Jul 01 18:48:25 2016 Limit ScrollAnchor to 20 adjustments between user scrolls. This somewhat mitigates the impact of pathological feedback loops with scroll event handlers. BUG= 601906 Review-Url: https://codereview.chromium.org/2118683002 Cr-Commit-Position: refs/heads/master@{#403491} [add] https://crrev.com/34ae0389f0c660c23f691595341188acc5444e00/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html [modify] https://crrev.com/34ae0389f0c660c23f691595341188acc5444e00/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp [modify] https://crrev.com/34ae0389f0c660c23f691595341188acc5444e00/third_party/WebKit/Source/core/layout/ScrollAnchor.h
,
Jul 7 2016
This still looks janky but no longer gets stuck forever. I don't know anything else that can be done here, so closing. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by ymalik@chromium.org
, Apr 8 2016