Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Physical items can grow unbounded when scrolling heterogeneously-sized list #559

Closed
1 of 8 tasks
kevinpschaaf opened this issue Oct 9, 2019 · 0 comments
Closed
1 of 8 tasks

Comments

@kevinpschaaf
Copy link
Contributor

kevinpschaaf commented Oct 9, 2019

Description

iron-list can suffer from a runaway condition where _increasePoolIfNeeded can loop indefinitely due to the physical average becoming large enough (due to heterogeneously-sized items) to position the physical top lower than the scroll position, causing _isClientFull to return true.

Expected outcome

Count of physical items never grows significantly larger than needed to cover the viewport.

Actual outcome

Count of physical items grows close to the virtual size of a large virtual list.

Live Demo

https://glitch.com/edit/#!/circular-breeze?path=index.html

Steps to reproduce

  1. Create list with large items array and heterogeneously-sized items
  2. Scroll to a point in the list such that the physical average becomes larger than at the start of the list
  3. Scroll back to the top of the list; if the physical average is significantly large enough, _scrollHandler will reset the _physicalTop to a value larger than the _scrollPosition.
  4. Fire an iron-resize event at the list. This will cause _increasePoolIfNeeded to check _isClientFull, which will return falsey because the items don't cover the viewport, and will loop indefinitely.

This bug appears to have been at least partially introduced in https://github.com/PolymerElements/iron-list/pull/435/files, where the _physicalTop is re-calculated purely as a function of physicalAverage, losing a guarantee that the _physicalTop is no longer above the start of the scroll position.

This could also explain some reported issues in #506 (although the OP of that issue seems to have been a user error with list sizing, others report similar runaway physical item creation behavior with a correctly sized list). I created a separate issue here since a GIF posted in that issue appears to have uniformly-sized items, and I believe this particular bug only occurs when the _physicalAverage used for estimation random-access scrolling becomes significantly larger than the physical items in the viewport.

Browsers Affected

  • Chrome (only confirmed on Chrome; didn't test other browsers)
  • Firefox
  • Safari 9
  • Safari 8
  • Safari 7
  • Edge
  • IE 11
  • IE 10
kevinpschaaf added a commit that referenced this issue Oct 10, 2019
…559

Ensure the _physicalTop is never recalculated to be below the current scroll position, and make sure cached scroll positions are used in methods which may be called outside of a scroll event handler for internal consistency.
kevinpschaaf added a commit that referenced this issue Oct 11, 2019
…ems-1.x

[1.x] Fix issue where _increasePoolIfNeeded could loop indefinitely. Fixes #559
kevinpschaaf added a commit that referenced this issue Oct 11, 2019
[3.x] Fix issue where _increasePoolIfNeeded could loop indefinitely. Fixes #559
kevinpschaaf added a commit that referenced this issue Oct 11, 2019
…559

Ensure the _physicalTop is never recalculated to be below the current scroll position, and make sure cached scroll positions are used in methods which may be called outside of a scroll event handler for internal consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant