I can only repro this on Mac (also tried to repro on Linux / Android with no luck). Can you confirm that you also observed this on Mac?

It may have something to do with our distinct implementation of ScrollAnimator on Mac. https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm&l=26

Will look into it more.

This is not mac specific.

There is a social div on the left which is stuck on the viewport. The way it is stuck to the viewport is by setting setting top to exactly the amount that the user scrolls by.

We initially anchor to it and as the user scrolls past it, JS on the page changes it's position to be in the viewport again. ScrollAnchor sees that it's anchor element moved, and adjust in response to it. This continues until we are all the way at the bottom of the document.

Repro: https://jsbin.com/pulaye/

Recapping whiteboard discussion with ojan and esprehn:

- Previously proposed solutions of detecting counterscroll, or having findAnchor avoid position:absolute with non-zero left/top, are too hacky / one-off.

- New proposal: avoid elements that are fully contained in the viewport.

- Assumption 1: emulated fixed/sticky positioning will satisfy this property, and not try to fix things where they are partially obscured.

- Assumption 2: there is usually or always a suitable partially-contained anchor node, such that avoiding the fully-contained elements will not make scroll anchoring less effective.

- Caveat: when scroll event handler reacts to a user scroll, ScrollAnchor::save() sees an intermediate state (page is scrolled, but the counterscrolling layout pass hasn't run yet).  In this intermediate state, the problem node is *not* fully in the viewport.  So it is not simply a matter of avoiding fully-contained nodes in save().

- Instead we want some notion of "where was this node before the user scroll".  Possibly this can be achieved by remembering the scroll position from the last restore().

- Another proposal to avoid the intermediate-state problem is to tie save/restore to the start/end of an animation frame, instead of the start/end of a layout pass.

I don't think remembering the last scroll position works. We are effectively running the algorithm as if the viewport is where it was before the user scroll which means we will pick a bad anchor. 

For example, say that the social div is near the top but fully contained in the viewport and the user hits the spacebar. When ScrollAnchor::save is called, the social div will be way above the viewport. When we are picking an anchor in ::save, we will detect that the social div was in the viewport before the scroll, but this also means that we will pick an anchor above the social div, which is way to high up on the page to be a good anchor. Does this make sense or am I missing something?

Yeah, this is tricky.

I think the idea may have been to skip the adjustment if the anchor node was fully visible after the previous layout (restore) AND fully visible after the current one.  So an element that isn't moved back into the viewport by the layout is fair game.

Of course by the time we know this it's too late to go back and pick a different anchor, so we'd be effectively disabling scroll anchoring in these cases.

But it's actually worse than that - if the user has scrolled since the last layout and the current layout is *not* a reaction to that scroll, then "viewport location after previous layout" is totally arbitrary and we should not be suppressing adjustments based on it.

If we only do this when the user *hasn't* scrolled since the last layout, we may be able to break the feedback loop by not adjusting for layouts that are reacting to previous adjustments.  But we'll still make a bad adjustment after every user scroll, if we anchor to an emulated-fixed element.

Tracking the user scroll delta that was applied in the current animation frame might help, but this assumes the page reacts immediately.  It will break if there is a rAF between the scroll handler and the layout invalidation, or if there is some animation happening like in  issue 601906 .

Let's land the patch to skip position:absolute with non-zero left/top for now.  If we figure out a better solution we can revisit.

I think it's fine to do as suggested in comment #9. Although, I still suspect the proposal in comment #6 might work.

Comment #7 is correct in that we're running the algorithm as if the viewport was where it was before the scroll, but I don't think that will result in bad anchor. The anchor would only be bad if something in between the old viewport and the new viewport changed size in that same frame. It's certainly possible, but I suspect it's pretty rare.

In either case, lets start simple as suggested in comment #9 and get to a point where we have a shippable thing. We can always extend/refine the cases where scroll anchoring works later. Let's keep the suggested solution from #6 in a list of V2 refinement ideas somewhere though so it doesn't get lost.

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

commit 556834f09ba3e4caf3ac682330df55c03bf0ffda
Author: ymalik <ymalik@chromium.org>
Date: Wed Jun 01 16:33:30 2016

Exclude position:absolute elements when picking an anchor for ScrollAnchoring

Only exclude ones that have top/left/right/bottom set to a non-zero value.

BUG= 604996 

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

[modify] https://crrev.com/556834f09ba3e4caf3ac682330df55c03bf0ffda/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
[modify] https://crrev.com/556834f09ba3e4caf3ac682330df55c03bf0ffda/third_party/WebKit/Source/core/layout/ScrollAnchorTest.cpp

 Issue 626249  has been merged into this issue.

This is fixed on the original URL.  Scroll anchoring is even performing useful adjustments on it. :)

Issue 617071 has been merged into this issue.