Bugzilla

(In reply to Botond Ballo [:botond] from comment #16)
> It's not immediately clear me how to fix this.

I thought about this some more, and realized that changing the initial value of `mLastScrollOrigin` to `nsGkAtoms::restore` is the wrong approach for fixing the problem. Bug 1304689 already handles correct restoration of the origin across a frame reconstruction with the `mAllowScrollOriginDowngrade` mechanism. The problem is just that if `ScrollToImpl()` early-exits [1], we don't execute the scroll origin downgrade logic [2].

Prior to these patches, that wasn't a problem, since if we're early-exiting `ScrollToImpl()`, the layout scroll offset is already what we want it to be, so it doesn't matter whether or not we downgrade the origin to `restore` or not. With these patches, though, it's important that we still downgrade the origin to `restore` so that a *visual* `restore` update riding the same transaction doesn't get clobbered.

[1] https://searchfox.org/mozilla-central/rev/01b4b3830ea3cae2e9e431019afa6391b471c6da/layout/generic/nsGfxScrollFrame.cpp#2710
[2] https://searchfox.org/mozilla-central/rev/01b4b3830ea3cae2e9e431019afa6391b471c6da/layout/generic/nsGfxScrollFrame.cpp#2760

(In reply to Botond Ballo [:botond] from comment #16)
> It's not immediately clear me how to fix this.

I thought about this some more, and realized that changing the initial value of `mLastScrollOrigin` to `nsGkAtoms::restore` is the wrong approach for fixing the problem. Bug 1304689 already handles correct restoration of the origin across a frame reconstruction with the `mAllowScrollOriginDowngrade` mechanism. The problem is just that if `ScrollToImpl()` early-exits [1], we don't execute the scroll origin downgrade logic [2].

Prior to these patches, that wasn't a problem, since if we're early-exiting `ScrollToImpl()`, the layout scroll offset is already what we want it to be, so it doesn't matter whether or not we downgrade the origin to `restore`. With these patches, though, it's important that we still downgrade the origin to `restore` so that a *visual* `restore` update riding the same transaction doesn't get clobbered.

[1] https://searchfox.org/mozilla-central/rev/01b4b3830ea3cae2e9e431019afa6391b471c6da/layout/generic/nsGfxScrollFrame.cpp#2710
[2] https://searchfox.org/mozilla-central/rev/01b4b3830ea3cae2e9e431019afa6391b471c6da/layout/generic/nsGfxScrollFrame.cpp#2760