1520666 - Ebay jumps to the top once you scroll to the bottom on a mobile device

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect, P2)

Description

[Stefan [:StefanG_QA]]

Mozilla/5.0 (Android 8.1.0; Mobile; rv:66.0) Gecko/66.0 Firefox/66.0 (20190116093310)

Tested with today's Nightly build on a Note 9 Android device.

STR:

1. Launch Firefox on a mobile device
2. Go to https://www.ebay.com/deals/daily/all
3. Scroll to the bottom
4. Observe

ER: Solid scroll position
AR: Once scrolled to the bottom of the page, the scroll bounce back to the top or the middle of the page.

Comment 1

[Timothy Nikkel (:tnikkel)]

Scroll anchoring related?

Comment 2

[Stefan [:StefanG_QA]]

I can't reproduce it with layout.css.scroll-anchoring.enabled set to false.

Comment 3

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Stefan [:StefanG_QA] from comment #0)

Mozilla/5.0 (Android 8.1.0; Mobile; rv:66.0) Gecko/66.0 Firefox/66.0
(20190116093310)

Tested with today's Nightly build on a Note 9 Android device.

STR:

1. Launch Firefox on a mobile device
2. Go to https://www.ebay.com/deals/daily/all
3. Scroll to the bottom
4. Observe

ER: Solid scroll position
AR: Once scrolled to the bottom of the page, the scroll bounce back to the
top or the middle of the page.

Can you still reproduce this now that bug 1519462 has landed?

Comment 4

[Stefan [:StefanG_QA]]

I'm still able to reproduce the bounce back to the top or middle of the page with today's Nightly build (01-17-2019) on an Android device.

Comment 5

[Ryan Hunt [:rhunt] (on leave until early May)]

I can reproduce this on my android device. Getting a build going for more information.

Comment 6

[Ryan Hunt [:rhunt] (on leave until early May)]

I kind of understand what's going on. Ebay is using infinity.js [1] here for a virtual list implementation.

infinity.js works by creating 'pages' of web content that are roughly the height of the viewport. Those pages are added and removed to the DOM as you scroll along. Above and below the visible pages are empty divs with the height of the content that would be above and below the viewport, respectively.

During a scroll, this process actually works okay with scroll anchoring. The problem is with resizing the window. When that happens, infinity.js attempts to 'repartition' the pages so each page still contains content roughly the height of the viewport. This process doesn't work well with scroll anchoring. I only have fuzzy ideas why, but I think it's for spec compliant reasons.

The reason we trigger this on Mobile and not Desktop is because of the dynamic toolbar. If you disable that with 'browser.chrome.dynamictoolbar', we seem to not scroll back up anymore.

You can see this on Nightly Desktop by going to the demo page [2], scrolling down a bit, and resizing the height of the window with the bottom edge.

The reason I believe that the 'repartition' step might not work with spec'ed scroll anchoring, is because I'm able to duplicate the same behavior by resizing the window on Chrome Desktop.

[1] https://airbnb.io/infinity/docs/infinity.html
[2] https://airbnb.io/infinity/demo-on.html

Comment 7

[Ryan Hunt [:rhunt] (on leave until early May)]

Also as an aside, we terminate pan momentum scrolls in APZ on OSX when we receive a relative update. This affects the ebay page a bit on desktop.

The problem is the code path for preserving animations doesn't work when we're just receiving a stream of pan momentum inputs [1]. In that case, setting the pan zoom state to 0, will cause us to ignore them and effectively stop momentum scrolling.

[1] https://searchfox.org/mozilla-central/rev/dac799c9f4e9f5f05c1071cba94f2522aa31f7eb/gfx/layers/apz/src/AsyncPanZoomController.cpp#4475

Comment 8

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Ryan Hunt [:rhunt] from comment #7)

Also as an aside, we terminate pan momentum scrolls in APZ on OSX when we
receive a relative update. This affects the ebay page a bit on desktop.

The problem is the code path for preserving animations doesn't work when
we're just receiving a stream of pan momentum inputs [1]. In that case,
setting the pan zoom state to 0, will cause us to ignore them and
effectively stop momentum scrolling.

[1]
https://searchfox.org/mozilla-central/rev/
dac799c9f4e9f5f05c1071cba94f2522aa31f7eb/gfx/layers/apz/src/
AsyncPanZoomController.cpp#4475

It looks like that's bug 1474196.

Comment 9

[Ryan Hunt [:rhunt] (on leave until early May)]

I wrote a reduced demo here [1]. You should be able to reproduce the issue by scrolling down and resizing the window.

The repartition algorithm works like this

newPageList = []
newPage = new Page()
// new Page() creates an empty unattached div

for page in oldPageList:
  for item in page:
    // this condition flushes layout
    if newPage.height + item.height > document.documentElement.clientHeight:
      newPageList += newPage
      newPage = new Page()
    newPage.append(item)
  // all items in page are removed from DOM
  page.remove()

for page in newPageList:
  page.insert()
  // all items in page are added to DOM

The problem is in the layout flush during the algorithm.

Say the user has scrolled down so they are anchored to the second page in the list. When this algorithm runs, the first iteration will remove the page above the anchor page. Then the second iteration will run, and flush layout. During this layout flush we'll scroll upwards because the anchor page is now the first page in the DOM.

This layout flush also affects us with scroll anchoring disabled, but not as severely. In that case, we need to be scrolled near the bottom of the page, so that the scroll position is clamped by the flushes.

The call to document.documentElement.clientHeight is done by jquery when executing $(window).height(). It's not clear to me that we need to flush in this situation, as I think we can get the current bounds (in the non-iframe case) without flushing. I hacked a patch that does this, and it fixes this test case.

This isn't the perfect solution, but it would fix Fennec's dynamic toolbar. Unfortunately this case is not handled by us, Chrome, or the spec. We only see it because we resize the viewport with the dynamic toolbar on Mobile.

[1] https://eqrion.github.io/web-tests/scrolling/infinite-js.html

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Should we only apply scroll adjustments on flushes that come from the refresh driver? It's not clear to me given the above it would help, not sure.

Comment 11

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Should we only apply scroll adjustments on flushes that come from the
refresh driver? It's not clear to me given the above it would help, not sure.

No, we can't do that or none of the WPTs would pass. Maybe we could rewrite them, but it'd be a fairly signficant implementation difference from Chrome. Which I'd like to avoid so we don't hit new web-compat issues.

It's unfortunate though that this relies on something unspecified like layout flushes, but it's the way Chrome's implementation works.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Is there a spec issue on file for that? Generally Chrome didn't want to expose when layout happened in a bunch of places (https://bugs.chromium.org/p/chromium/issues/detail?id=460822 comes to mind)...

Comment 13

[Ryan Hunt [:rhunt] (on leave until early May)]

No spec issue yet, but here's the relevant parts of the spec [1] [2]. Feel free to file, I'm not sure I'd have a great alternative to propose.

[1] https://drafts.csswg.org/css-scroll-anchoring-1/#scroll-adjustment
[2] https://drafts.csswg.org/css-scroll-anchoring-1/#suppression-window

Comment 14

[Ryan Hunt [:rhunt] (on leave until early May)]

Emilio looked into possibly not flushing layout on accessing 'document.documentElement.clientHeight', and it's not feasible.

It also sounds like the dynamic toolbar code is something that will change with GeckoView, and there's a chance we might stop resizing. But I don't know if we should ship with a regression like this now. Especially because this feature is supposed to make mobile better.

I'm looking into adding a hack to not perform scroll adjustments while we are resizing do to a dynamic toolbar adjustment.

Comment 15

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Ryan Hunt [:rhunt] from comment #14)

Emilio looked into possibly not flushing layout on accessing
'document.documentElement.clientHeight', and it's not feasible.

It also sounds like the dynamic toolbar code is something that will change
with GeckoView, and there's a chance we might stop resizing. But I don't
know if we should ship with a regression like this now. Especially because
this feature is supposed to make mobile better.

I'm looking into adding a hack to not perform scroll adjustments while we
are resizing do to a dynamic toolbar adjustment.

...

And that won't work either because the page throttles the resize events using timeouts. So there's no connection between when we fire resize events for a dynamic toolbar resize, and when the repartition code fires.

I'm not sure what we can do to fix this on the scroll anchoring side.

Comment 16

[Ryan Hunt [:rhunt] (on leave until early May)]

Also, I was going to open a PR against 'infinity.js' to cache the window height to avoid this flush, but it appears that it has been deprecated [1].

[1] https://github.com/airbnb/infinity

Comment 17

[Emilio Cobos Álvarez (:emilio)]

James, do you know how hard it is to avoid resizing the website's viewport in Fennec when showing / hiding the toolbar, like Focus does?

It causes a bunch of hazards like this and bug 1512872, which don't happen in other mobile browsers.

Comment 18

[Hiroyuki Ikezoe (:hiro)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

It causes a bunch of hazards like this and bug 1512872, which don't happen in other mobile browsers.

FYI: https://github.com/bokand/bokand.github.io/blob/master/web_viewports_explainer.md#mobile-browser-ui-interactions

Comment 19

[Ryan Hunt [:rhunt] (on leave until early May)]

In the short-term I think we will work around this by disabling on Fennec and riding the trains without it.

Comment 20

[Ryan Hunt [:rhunt] (on leave until early May)]

An update. Scroll anchoring was disabled in bug 1515946 on Fennec to fix this issue (but not GeckoView as it's unaffected). So no one should be experiencing this issue.

We'd still really like to keep it enabled in Fennec for mobile testing, so we're looking at options to fix this. Which is why I'm still going to keep this bug open.

Comment 21

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

James, do you know how hard it is to avoid resizing the website's viewport in Fennec when showing / hiding the toolbar, like Focus does?

It causes a bunch of hazards like this and bug 1512872, which don't happen in other mobile browsers.

It's a big change, but probably doable. IIRC, the resizing is done to accomodate height=device-height pages that would otherwise half a toolbar's worth of height hanging off the bottom of the page if we didn't resize. For GeckoView-based apps we have talked about just pinning the toolbar in those cases and never resizing. We could probably do that for Fennec too.

Comment 22

[James Willcox (:snorp) ([e-mail]) (he/him)]

NI botond and rbarker who know the Fennec dynamic toolbar bits far better than I.

Comment 23

[Randall Barker [:rbarker]]

Chrome resized the viewport to hide the toolbar. The whole point of the dynamic tool bar was to ensure fixed page elements stayed on screen and did not jitter during scrolling.

Comment 24

[James Willcox (:snorp) ([e-mail]) (he/him)]

Yeah I just tested here and Chrome does appear to resize.

Comment 25

[Ryan Hunt [:rhunt] (on leave until early May)]

This might be relevant then [1]. Looking into it more.

[1] https://github.com/WICG/ScrollAnchoring/issues/11

Comment 26

[Botond Ballo [:botond]]

I don't have a good specific understanding of what not resizing the viewport with the current Fennec dynamic toolbar implementation would entail, but based on my recollection of the size and complexity of the dynamic toolbar patches, it seems like a significant amount of work.

As we're already reworking how the dynamic toolbar works for GeckoView, I'd rather avoid doing it again for Fennec. If that means some newer features like scroll anchoring work in GeckoView and not Fennec, perhaps that's an acceptable tradeoff.

Comment 27

[Daniel Holbert [:dholbert]]

For folks who can reproduce (with scroll anchoring reenabled in about:config): do you see the bug at the following URLs as well?

https://www.ebay.co.uk/deals/trending/all
https://www.ebay.ca/deals/trending/all
https://www.ebay.co.uk/deals/featured/final-clearance-extra-15-off-your-favourite-brands

Those all have enough content that I would expect that they'd be able to trigger it (and if so, we'd want bug 1522755's intervention to be broader than just .com)...

But I can't seem to reproduce the bug at all on my Pixel 3 (even on the .com version of ebay, with comment 0's URL, after preffing on scroll anchoring). So I can't tell if these .co.uk URLs might be affected or not.

Comment 28

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Daniel Holbert [:dholbert] from comment #27)

For folks who can reproduce (with scroll anchoring reenabled in
about:config): do you see the bug at the following URLs as well?

https://www.ebay.co.uk/deals/trending/all
https://www.ebay.ca/deals/trending/all

https://www.ebay.co.uk/deals/featured/final-clearance-extra-15-off-your-favourite-brands

Those all have enough content that I would expect that they'd be able to
trigger it (and if so, we'd want bug 1522755's intervention to be broader
than just .com)...

But I can't seem to reproduce the bug at all on my Pixel 3 (even on the .com
version of ebay, with comment 0's URL, after preffing on scroll anchoring).
So I can't tell if these .co.uk URLs might be affected or not.

I can't seem to get enough content to reproduce the bug on those domains.

The virtual scroller doesn't start to trigger until you've reached near the
bottom and more content dynamically loads. Or if you scroll fast enough and
click the "load more" button.

I haven't seen that on those domains. Not sure why they don't have as much
content.

But regardless, that's a great point and we definitely need to have the
intervention apply here. I can confirm these pages are still loading
'infinity.js' with the problematic 'hasVacancy' function.

Comment 29

[Daniel Holbert [:dholbert]]

:denschub confirmed that he was able to reproduce on one of those (over in https://phabricator.services.mozilla.com/D17600#451783 )

So, that (combined with rhunt's observations about the problematic JS being loaded) gives us confirmation that those country-specific-TLD versions are affected in some cases as well. Which is fine, because the intervention in bug 1522755 will now cover all known ebay.[whatever], sites, it looks like.

Comment 30

[Ryan Hunt [:rhunt] (on leave until early May)]

I've updated my test case here to isolate the problematic 'repartition' code [1]. If you click anywhere on the page, we should run the function directly (no resize) and change the background color (to verify it happened).

To reproduce the issue, scroll halfway down the page and click. I can reproduce the issue on Firefox Desktop, Firefox for Android, and Chrome Desktop, but not on Chrome for Android, or Chrome Desktop in responsive design mode.

Looking at Chrome's code, here's how they emulate mobile for responsive design mode [2]. It seems they set a bunch of runtime flags, and reformat. The flag that jumps out to me is that they force overlay scrollbars.

In their element.clientHeight implementation [3], it looks like they special case being the documentElement, with overlay scrollbars, and being a local frame root (not an embedded iframe). In this case they avoid doing a layout flush, which would trigger scroll anchoring.

I believe this is the reason that Chrome doesn't experience this issue on mobile.

Emilio, we talked about not flushing here briefly and it seemed like it wasn't possible. Can we do something similar here, or is this a spec issue?

[1] https://eqrion.github.io/web-tests/scrolling/infinite-js.html
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/dev_tools_emulator.cc?l=290&rcl=e95660c6c08b9b855ceebaf21a1614bc3df75ff8
[3] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/element.cc?l=735&rcl=e95660c6c08b9b855ceebaf21a1614bc3df75ff8

Comment 31

[Emilio Cobos Álvarez (:emilio)]

Ah, I see. So what I missed when I was testing this is that this differs from the quirks and non-quirks-mode cases. I think Blink is buggy there since in quirks mode the body is only the scrolling element if it itself is not scrollable. Looks like they don't account for that special-case which seems like a bug to me...

But yeah, looks like we could do something like avoid flushing layout if we have overlay scrollbars and OwnerDoc()->IsScrollingElement(this) returns true. I'd run that through Boris though just in case.

Comment 32

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: scrolling-element-no-flush.patch

I ended up just experimenting with a patch to do this.

Boris, does this look correct? I'm unfamiliar with what's required here for compatibility.

Comment 33

[Ryan Hunt [:rhunt] (on leave until early May)]

Just to add, I think this bug is a good example of why scroll anchor adjustments probably shouldn't be triggered by layout flushes. To my knowledge, there's no spec on what causes layout flushes and this will probably cause other interop issues.

Changing it for the initial release doesn't seem likely though, so a fix like this seems easier.

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 9039615 [details] [diff] [review]
scrolling-element-no-flush.patch

So per spec, clientHeight is supposed to have the following behavior:

1) Return 0 if there is no CSS layout box.  This requires at least a frames/style flush.  That said, it doesn't look like we implement that behavior, nor do Chrome or Safari.  So maybe we can fix this part in the spec?  Emilio?

2) Return the size of the viewport minus scrollbars if the element is root or body, depending on quirkiness.

3) Return stuff that needs to flush layout otherwise.

If we ignore item #1, restrict to overlay scrollbars (so we don't have to do layout to deal with those) and stick to the elements that map to the viewport, then we can in fact avoid flushing layout here.  

IsScrollingElement() looks like the wrong check in quirks mode with a non-scrollable body.  Though I guess the failure mode would just be that we fall through to the flushing path, right?

As far as the actual value being returned, should this conceptually differ from what nsGlobalWindowOuter::GetInnerSize does?  It looks to me like the main difference is whether we consider presShell->IsVisualViewportSizeSet().  Should we be considering it here?  Ideally we would just share this code between the two places or something...

Comment 35

[Emilio Cobos Álvarez (:emilio)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #34)

Comment on attachment 9039615 [details] [diff] [review]
scrolling-element-no-flush.patch

So per spec, clientHeight is supposed to have the following behavior:

1. Return 0 if there is no CSS layout box. This requires at least a
   frames/style flush. That said, it doesn't look like we implement that
   behavior, nor do Chrome or Safari. So maybe we can fix this part in the
   spec? Emilio?

Note that I'm not a cssom-view editor, but sure, if two implementations (and probably WebKit too) agree, the spec should probably be fixed.

Filed https://github.com/w3c/csswg-drafts/issues/3582.

2. Return the size of the viewport minus scrollbars if the element is root
   or body, depending on quirkiness.

3. Return stuff that needs to flush layout otherwise.

If we ignore item #1, restrict to overlay scrollbars (so we don't have to do
layout to deal with those) and stick to the elements that map to the
viewport, then we can in fact avoid flushing layout here.

IsScrollingElement() looks like the wrong check in quirks mode with a
non-scrollable body. Though I guess the failure mode would just be that we
fall through to the flushing path, right?

As far as the actual value being returned, should this conceptually differ
from what nsGlobalWindowOuter::GetInnerSize does? It looks to me like the
main difference is whether we consider presShell->IsVisualViewportSizeSet().
Should we be considering it here? Ideally we would just share this code
between the two places or something...

Probably yes, it should differ. clientWidth should return IIRC the layout viewport, but innerWidth / innerHeight should return the visual viewport. Though Hiro or Botond can probably confirm / sanity-check that, haven't been following the viewport stuff too closely.

Comment 36

[Hiroyuki Ikezoe (:hiro)]

Botond is the right person to be asked, but IIUC, as of now clientWidth is not the layout viewport (fixed viewport) width, it's ICB. It ideally should be fixed viewport, but Bokand worries that changing it breaks sites in the wild, see this discussion.

(Botond, correct me if I am wrong, the terminologies there are pretty confusing to me)

Comment 37

[Botond Ballo [:botond]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #36)

IIUC, as of now clientWidth is not the layout viewport (fixed viewport) width, it's ICB.

That's right.

It ideally should be fixed viewport, but Bokand worries that changing it breaks sites in the wild, see this discussion.

I'm not actually sure what it should be ideally. I initially thought the layout viewport based on a reading of the spec, but as the spec doesn't actually differentiate between the different viewport-like quantities, that doesn't mean much.

Presumably, it's useful for a website author to have a way of querying the initial containing block size, and if clientWidth is that way, perhaps it should remain so.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #35)

innerWidth / innerHeight should return the visual viewport

innerWidth/innerHeight currently return visual viewport dimensions in Firefox, but we'd like to move to them returning layout (fixed) viewport dimensions, as Chrome does; bug 1514429 tracks.

Comment 38

[Tim Spurway [:tspurway]]

are we planning on landing this for 66, ryan?

Comment 39

[Ryan Hunt [:rhunt] (on leave until early May)]

Just had a discussion with Botond about this on irc. It sounds like our behavior with clientWidth/clientHeight regressed to use the fixed viewport in bug 1525075.

The attached patch here will return the ICB when the heuristic conditions are met, causing a behavior difference. As returning the ICB is the desired solution, it sounds like bug 1525075 should be fixed first. Or optionally both issues fixed with the same patch.

Comment 40

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1520666 - Avoid flushing layout when accessing clientArea for the document scrolling element in certain cases. r?botond

This commit attempts to prevent a layout flush when accessing clientWidth
or clientHeight on an element and the following conditions are met:

1. The document is not embedded in another document
2. The element is the scrolling element of the document
3. We are using overlay scrollbars

This matches a similar optimization in Chromium [1].

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/element.cc?l=735&rcl=e95660c6c08b9b855ceebaf21a1614bc3df75ff8

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be1132e03145
Avoid flushing layout when accessing clientArea for the document scrolling element in certain cases. r=botond

Comment 42

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/be1132e03145

Comment 43

[Liz Henry (:lizzard) (relman/hg->git project)]

Would you like to request uplift? Since this is a new regression in 66 we have a chance to fix it before shipping.

Comment 44

[Ryan Hunt [:rhunt] (on leave until early May)]

I was hoping to uplift this after bug 1529765 landed, but I guess we don't need to wait.

Comment 45

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 9044332 [details]
Bug 1520666 - Avoid flushing layout when accessing clientArea for the document scrolling element in certain cases. r?botond

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Scroll Anchoring
• User impact if declined: Sites that use the infinity.js script for virtual scrollers that aren't covered by the gofaster intervention may experience abrupt jumps back up the page when scrolling down on mobile.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1525075
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Avoids a layout flush in a very specific case that affects some pages. Chrome has had a similar optimization for a while.
• String changes made/needed:

Comment 46

[Eliza Balazs]

Hi!
Verified as fixed on latest Nightly build (67.0a1) with Samsung Galaxy Note 8 (Android 8.0.0).

Comment 47

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9044332 [details]
Bug 1520666 - Avoid flushing layout when accessing clientArea for the document scrolling element in certain cases. r?botond

Fix for new regression in 66, verified in Nightly.
OK for beta 13 uplift.

Comment 48

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fb041ef6b3bf

Comment 49

[Andrei Bodea[:andrei]]

Re-opened in Firefox 66.0b13.
Note that I was able to reproduce this issue on the latest Beta build 66.0b13 following the steps from the description (Comment 0).
Also, note that I can reproduce this issue only first time when opening https://www.ebay.com/deals/daily/all. The issue can be reproduce every time when opening a new tab with https://www.ebay.com/deals/daily/all.
After reproducing the issue on a new tab and scrolling up and down, the issue cannot be reproduced until opening / refreshing the tab.
I was able to reproduce this issue on the following devices: Samsung Galaxy S9(Android 8.0.0), Samsung Galaxy J7(Android 6.0.1), Google Pixel 3XL Android P, Samsung Galaxy Note 9(Android 8.1.0).
But on the following devices the issue could not be reproduced: Google Pixel (Android 9), Nexus 6P(Android 8.1.0) and Nokia 6(Android 7.1.1)
I was able to reproduce this issue on the latest Nightly build too.
A very important note would be that when I scrolled down, the website expanded once but if you keep scrolling when reaching the bottom, the screen jumps to the middle of the page(as described in Comment 0).
For more details please see the following attachments: In this Video i reproduced the issue 3 times, every time with a new page and I have attached also a logcat exactly when I reproduced the issue more than 2 times.
Stefan can you please confirm that after reading my comment and watching the video this is exactly how you reproduced the issue?

Thanks,
Andrei

Comment 50

[Stefan [:StefanG_QA]]

After reading your comment and watching the video I can confirm this is the way I used to reproduce it.

Comment 51

[Liz Henry (:lizzard) (relman/hg->git project)]

Ryan, can you take another look? Thanks.

Comment 52

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Andrei Bodea[:andrei] from comment #49)

Re-opened in Firefox 66.0b13.
Note that I was able to reproduce this issue on the latest Beta build
66.0b13 following the steps from the description (Comment 0).
Also, note that I can reproduce this issue only first time when opening
https://www.ebay.com/deals/daily/all. The issue can be reproduce every time
when opening a new tab with https://www.ebay.com/deals/daily/all.
After reproducing the issue on a new tab and scrolling up and down, the
issue cannot be reproduced until opening / refreshing the tab.
I was able to reproduce this issue on the following devices: Samsung Galaxy S9(Android 8.0.0), Samsung Galaxy J7(Android 6.0.1), Google Pixel 3XL Android P, Samsung Galaxy Note 9(Android 8.1.0).
But on the following devices the issue could not be reproduced: Google Pixel (Android 9), Nexus 6P(Android 8.1.0) and Nokia 6(Android 7.1.1)
I was able to reproduce this issue on the latest Nightly build too.
A very important note would be that when I scrolled down, the website
expanded once but if you keep scrolling when reaching the bottom, the screen
jumps to the middle of the page(as described in Comment 0).
For more details please see the following attachments: In this
[Video](https://drive.google.com/file/d/1_jppOLLC18rd88KZk4-lcz8_9ZwmpEDP/
view) i reproduced the issue 3 times, every time with a new page and I have
attached also a
[logcat](https://drive.google.com/file/d/1Pv2XGJGvpPqmvrk3n5Tnti-e2viQxaMG/
view?usp=sharing) exactly when I reproduced the issue more than 2 times.
Stefan can you please confirm that after reading my comment and watching the
video this is exactly how you reproduced the issue?

Thanks,
Andrei

Hi Andrei,

Thanks for taking a look at this.

I just took a look at the video you've provided (looked at it a couple times in 1x and 0.25x speed), and I'm not sure I'm seeing the issue that we were experiencing.

It looks like you reach the bottom of the page faster than it's able to load new content, and when that content is loaded you're then in the middle of the page, as new content has loaded below where you are.

I would usually trigger the issue by scrolling to the bottom of the page enough times that no new content would load, and then scrolling halfway up the page and then back down slightly. This would cause the scroll position to jump up significantly.

I only have a Google Pixel, so I wasn't able to reproduce it in the latest Firefox Beta.

Comment 53

[Andrei Bodea[:andrei]]

Hello Ryan,

The problem with that page is that it can be expendable many times. Normally when opening it, the page has a limited content that is loaded. You can see that in video or by trying this on your device, every time when opening a new page with that link the same content will be loaded. The problem is the scrolling is jumping to the middle instead of going to the bottom(note that I always scrolled to reach the bottom). In my video you can see that after the content is loaded and the the screen is redirected to the middle instead of bottom page.

As I said is very specific reproducing it only the first time when opening a page.
Stefan confirmed that after reading my comment this was the way he reproduced the issue in Comment 50.

Also, if you need any other information regarding this issue, please do not hesitate to contact me.

Thanks,
Andrei

Comment 54

[Ryan Hunt [:rhunt] (on leave until early May)]

I used mozregression to get one of the affected builds (60944b0fca8f) and made a screencast of the issue. [1]

You can see the issue at 21 seconds. The user has loaded the page and scrolls upward enough to have the toolbar become visible again. When this happens the screen jumps to a different position.

Are you able to reproduce that issue?

It looks like in your screencast the user scrolls to the bottom of the page, which causes new content to load, making what was the bottom of the page, now the middle of the page. I don't see the same scroll jumping that I've linked to in my screencast.

[1] https://drive.google.com/file/d/1Ju04_L9eNkmgtO3kDDbaacYfAHPCTKV7/view?usp=sharing

Comment 55

[Andrei Bodea[:andrei]]

Hello Ryan,

After I watched your video I can say that this is the way my Stefan's issue is reproducible but with a small correction, if you watch your video closely you still have show more option there displayed, after that you are redirected to the middle of the page. When you see my video the issue is almost the same but in my case everything happens a lot faster and I scrolled faster in order to reach the bottom of the page(just like your issue).
The products that were displayed when you reached the bottom of the page are overcome by other products which make that issue similar with how I was able to reproduced it(with Stefan confirmation) but in my case the webpage was loaded a lot faster.
I consider that in the case I presented even if we don't get to see the "bottom" page the scroll should still go down and not in the middle of the webpage.

Regards,
Andrei

Comment 56

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Andrei Bodea[:andrei] from comment #55)

Hello Ryan,

After I watched your video I can say that this is the way my Stefan's issue
is reproducible but with a small correction, if you watch your video closely
you still have show more option there displayed, after that you are
redirected to the middle of the page. When you see my video the issue is
almost the same but in my case everything happens a lot faster and I
scrolled faster in order to reach the bottom of the page(just like your
issue).
The products that were displayed when you reached the bottom of the page are
overcome by other products which make that issue similar with how I was able
to reproduced it(with Stefan confirmation) but in my case the webpage was
loaded a lot faster.
I consider that in the case I presented even if we don't get to see the
"bottom" page the scroll should still go down and not in the middle of the
webpage.

Regards,
Andrei

Hi Andrei,

I took another look at your video, and I don't think there's a bug here. When you scroll down to the bottom of the page, more products will load and you won't be at the bottom of the page anymore. The scroll position hasn't changed, there's just new content added to the page. This is the desired behavior of the page, and with scroll anchoring.

The original issue of this bug was that when the dynamic toolbar appeared or disappeared, it would cause the scroll position to jump upwards. This would happen even if you weren't at the bottom of the page. I cannot reproduce that behavior anymore.

So with that, I'm going to mark this as fixed again. Feel free to contact me with more information if necessary.