Closed Bug 1280330 Opened 6 years ago Closed 6 years ago

APZC cannot perform upward fling on google infinite-scrollbar demo

Categories

(Core :: Panning and Zooming, defect, P2)

48 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1268140
Tracking Status
firefox48 --- wontfix
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix

People

(Reporter: bkelly, Assigned: kats)

References

()

Details

(Whiteboard: [gfx-noted])

See this infinite scrolling list demo:

http://googlechrome.github.io/ui-element-samples/infinite-scroller/

Do a few fast downward flings so you are many pages down from the top.  You should see some dummy elements that get filled in over time.  This all works well.

If you then try to fling back up, the scrolling seems to get "stuck".  Its not really stuck because it does scroll a bit, but it immediately stops.

It seems like some kind of DOM manipulation performed by the page is confusing our APZC.

If you run this site with e10s disabled then the problem does not occur.  This is why I suspect APZC.
Flags: needinfo?(bugmail.mozilla)
WFM on Aurora on OS X. What platform/version are you seeing this on?
Flags: needinfo?(bugmail.mozilla) → needinfo?(bkelly)
I was able to reproduce it today magically, after seeing Ben demo it on his machine. It looks like something is triggering a scroll offset update to APZ, like the frame reconstruction stuff or maybe actual scrollTo/scrollTop setting.
Flags: needinfo?(bkelly)
(For the record we were also seeing this intermittently with e10s disabled on release, it's just much worse with APZ)
Priority: -- → P2
Whiteboard: [gfx-noted]
Doesn't look like we'll get to this for 48. Even if we do fix it it's likely too risky to uplift now.
I showed this to Rick Byers this evening and he wondered if we were hitting a bug when the original target node is removed from the DOM.  He theorized maybe we dropped the fling if the original target goes away.  Kats, what do you think?
Flags: needinfo?(bugmail)
While I wouldn't rule it out, I don't think that is what is going on here. I'm fairly sure this is a problem similar to ones we have seen before where layout does a frame reconstruction and we end up aborting the APZ fling as a result.
Flags: needinfo?(bugmail)
See Also: → 1247074
I tried an inbound build which has the fixes for both bug 1292781 and bug 1286179 but sadly I could still reproduce this issue. Will need to investigate.
Sorry meant to keep this affected on 50 so it stays on my radar for this release.
I can still repro this on a recent local build. I have time to investigate, finally!
Assignee: nobody → bugmail
Turns out this is caused by the infinite scroller setting scrollTop explicitly. That interrupts the fling animation that we're doing, so this not an APZ bug.

https://github.com/GoogleChrome/ui-element-samples/blob/gh-pages/infinite-scroller/scripts/infinite-scroll.js#L333
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
Kats, would you be willing to write an issue on that repo describing the incompatibility with firefox?  It seems this code works in other browsers, so there is some web compat issue here we need to sort.  Right?

https://github.com/GoogleChrome/ui-element-samples/issues
Flags: needinfo?(bugmail)
Kats, would it be possible to update our position when scrollTop is set, but still maintain the current fling momentum?
Flags: needinfo?(bugmail)
Anything is possible - it's just code. The question is really - what's the right thing to do here, and is it worth doing? If it were just this demo page (which per [1] is explicitly not intended to be compatible with all browsers) I couldn't care less. However I recall now that this has come up before in bug 1268140, and so we will probably will attempt to do this at some point. It's nontrivial though.

[1] https://github.com/GoogleChrome/ui-element-samples/issues/24
Flags: needinfo?(bugmail)
See Also: 12470741268140
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
> Turns out this is caused by the infinite scroller setting scrollTop
> explicitly. That interrupts the fling animation that we're doing, so this
> not an APZ bug.

Isn't this bug 1268140? In other words, I disagree that scrollTop is supposed to interrupt flings, and I think we should change this.
Oh, you mentioned that specific bug in your last comment. Sorry.
At the time I thought that the youtube bug was using scrollBy but it's not. So really this is just a straight up duplicate.
Resolution: INVALID → DUPLICATE
Duplicate of bug: 1268140
You need to log in before you can comment on or make changes to this bug.