GitHub/ReactWindow
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add guards to prevent scrollTo/scrollToItem from accepting incorrect values

Browse Source
This commit is contained in:
Brian Vaughn
2019-04-09 15:47:21 -07:00
parent 7175967ef0
commit 0b5a3d3cd0
12 changed files with 199 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+3
View File
@@ -1,6 +1,9 @@
Changelog
------------
### 1.7.2
* 🐛 Add guards to avoid invalid scroll offsets when `scrollTo()` is called with a negative offset or when `scrollToItem` is called with invalid indices (negative or too large).
### 1.7.1
* 🐛 Fix SSR regression introduced in 1.7.0 - ([Betree](https://github.com/Betree) - [#185](https://github.com/bvaughn/react-window/pull/185))
package.json
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "react-window",
"version": "1.7.1",
"version": "1.7.2",
"description":
"React components for efficiently rendering large, scrollable lists and tabular data",
"author":
src/__tests__/FixedSizeGrid.js
+42 -19
View File
@@ -431,31 +431,30 @@ describe('FixedSizeGrid', () => {
})
);
});
it('should ignore offsets less than zero', () => {
const onScroll = jest.fn();
const instance = ReactDOM.render(
<FixedSizeGrid {...defaultProps} onScroll={onScroll} />,
document.createElement('div')
);
instance.scrollTo({ scrollLeft: 100, scrollTop: 100 });
onScroll.mockClear();
instance.scrollTo({ scrollLeft: -1, scrollTop: -1 });
expect(onScroll.mock.calls[0][0].scrollLeft).toBe(0);
expect(onScroll.mock.calls[0][0].scrollTop).toBe(0);
});
});
describe('scrollToItem method', () => {
it('should not set invalid offsets when the list contains few items', () => {
const onScroll = jest.fn();
const rendered = ReactTestRenderer.create(
<FixedSizeGrid
{...defaultProps}
columnCount={1}
rowCount={2}
onScroll={onScroll}
/>
<FixedSizeGrid {...defaultProps} columnCount={1} rowCount={2} />
);
onScroll.mockClear();
// Offset should not be negative.
rendered
.getInstance()
.scrollToItem({ columnIndex: 0, rowIndex: 0, align: 'auto' });
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'backward',
scrollLeft: 0,
scrollTop: 0,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'backward',
});
expect(onItemsRendered).toMatchSnapshot();
onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -679,6 +678,30 @@ describe('FixedSizeGrid', () => {
verticalScrollDirection: 'backward',
});
});
it('should ignore indexes less than zero', () => {
const instance = ReactDOM.render(
<FixedSizeGrid {...defaultProps} />,
document.createElement('div')
);
instance.scrollToItem({ columnIndex: 20, rowIndex: 20 });
onItemsRendered.mockClear();
instance.scrollToItem({ columnIndex: -1, rowIndex: -1 });
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});
it('should ignore indexes greater than itemCount', () => {
const instance = ReactDOM.render(
<FixedSizeGrid {...defaultProps} />,
document.createElement('div')
);
onItemsRendered.mockClear();
instance.scrollToItem({
columnIndex: defaultProps.columnCount * 2,
rowIndex: defaultProps.rowCount * 2,
});
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});
});
// onItemsRendered is pretty well covered by other snapshot tests
src/__tests__/FixedSizeList.js
+36 -7
View File
@@ -363,6 +363,18 @@ describe('FixedSizeList', () => {
instance.scrollTo(100);
expect(itemRenderer.mock.calls[0][0].isScrolling).toBe(false);
});
it('should ignore values less than zero', () => {
const onScroll = jest.fn();
const instance = ReactDOM.render(
<FixedSizeList {...defaultProps} onScroll={onScroll} />,
document.createElement('div')
);
instance.scrollTo(100);
onScroll.mockClear();
instance.scrollTo(-1);
expect(onScroll.mock.calls[0][0].scrollOffset).toBe(0);
});
});
describe('scrollToItem method', () => {
@@ -371,14 +383,10 @@ describe('FixedSizeList', () => {
const rendered = ReactTestRenderer.create(
<FixedSizeList {...defaultProps} itemCount={3} onScroll={onScroll} />
);
onScroll.mockClear();
// Offset should not be negative.
expect(onItemsRendered).toMatchSnapshot();
onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
expect(onScroll).toHaveBeenCalledWith({
scrollDirection: 'backward',
scrollOffset: 0,
scrollUpdateWasRequested: true,
});
expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -457,6 +465,27 @@ describe('FixedSizeList', () => {
instance.scrollToItem(15);
expect(itemRenderer.mock.calls[0][0].isScrolling).toBe(false);
});
it('should ignore indexes less than zero', () => {
const instance = ReactDOM.render(
<FixedSizeList {...defaultProps} />,
document.createElement('div')
);
instance.scrollToItem(20);
onItemsRendered.mockClear();
instance.scrollToItem(-1);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});
it('should ignore indexes greater than itemCount', () => {
const instance = ReactDOM.render(
<FixedSizeList {...defaultProps} />,
document.createElement('div')
);
onItemsRendered.mockClear();
instance.scrollToItem(defaultProps.itemCount * 2);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});
});
// onItemsRendered is pretty well covered by other snapshot tests
src/__tests__/VariableSizeGrid.js
+10 -18
View File
@@ -159,18 +159,10 @@ describe('VariableSizeGrid', () => {
onScroll={onScroll}
/>
);
onScroll.mockClear();
// Offset should not be negative.
rendered
.getInstance()
.scrollToItem({ columnIndex: 0, rowIndex: 0, align: 'auto' });
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'backward',
scrollLeft: 0,
scrollTop: 0,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'backward',
});
expect(onItemsRendered).toMatchSnapshot();
onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -290,12 +282,12 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
.scrollToItem({ columnIndex: 15, rowIndex: 10, align: 'end' });
.scrollToItem({ columnIndex: 5, rowIndex: 10, align: 'end' });
// With hidden scrollbars (size === 0) we would expect...
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
scrollLeft: 720,
scrollLeft: 115,
scrollTop: 230,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'forward',
@@ -306,12 +298,12 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
.scrollToItem({ columnIndex: 15, rowIndex: 10, align: 'end' });
.scrollToItem({ columnIndex: 5, rowIndex: 10, align: 'end' });
// With scrollbars of size 20 we would expect those values ot increase by 20px
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
scrollLeft: 740,
scrollLeft: 135,
scrollTop: 250,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'forward',
@@ -352,13 +344,13 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
.scrollToItem({ columnIndex: 15, rowIndex: 0, align: 'end' });
.scrollToItem({ columnIndex: 5, rowIndex: 0, align: 'end' });
// Since there aren't enough rows to require vertical scrolling,
// the additional 20px for the scrollbar should not be taken into consideration.
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
scrollLeft: 720,
scrollLeft: 115,
scrollTop: 0,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'backward',
src/__tests__/VariableSizeList.js
+3 -7
View File
@@ -121,14 +121,10 @@ describe('VariableSizeList', () => {
const rendered = ReactTestRenderer.create(
<VariableSizeList {...defaultProps} itemCount={3} onScroll={onScroll} />
);
onScroll.mockClear();
// Offset should not be negative.
expect(onItemsRendered).toMatchSnapshot();
onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
expect(onScroll).toHaveBeenCalledWith({
scrollDirection: 'backward',
scrollOffset: 0,
scrollUpdateWasRequested: true,
});
expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
src/__tests__/__snapshots__/FixedSizeGrid.js.snap
+36
View File
@@ -273,6 +273,42 @@ Array [
]
`;
exports[`FixedSizeGrid scrollToItem method should ignore indexes greater than itemCount 1`] = `
Array [
Array [
Object {
"overscanColumnStartIndex": 97,
"overscanColumnStopIndex": 99,
"overscanRowStartIndex": 95,
"overscanRowStopIndex": 99,
"visibleColumnStartIndex": 98,
"visibleColumnStopIndex": 99,
"visibleRowStartIndex": 96,
"visibleRowStopIndex": 99,
},
],
]
`;
exports[`FixedSizeGrid scrollToItem method should ignore indexes less than zero 1`] = `
Array [
Array [
Object {
"overscanColumnStartIndex": 0,
"overscanColumnStopIndex": 3,
"overscanRowStartIndex": 0,
"overscanRowStopIndex": 5,
"visibleColumnStartIndex": 0,
"visibleColumnStopIndex": 2,
"visibleRowStartIndex": 0,
"visibleRowStopIndex": 4,
},
],
]
`;
exports[`FixedSizeGrid scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
exports[`FixedSizeGrid scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
src/__tests__/__snapshots__/FixedSizeList.js.snap
+28
View File
@@ -153,6 +153,34 @@ Array [
]
`;
exports[`FixedSizeList scrollToItem method should ignore indexes greater than itemCount 1`] = `
Array [
Array [
Object {
"overscanStartIndex": 94,
"overscanStopIndex": 99,
"visibleStartIndex": 96,
"visibleStopIndex": 99,
},
],
]
`;
exports[`FixedSizeList scrollToItem method should ignore indexes less than zero 1`] = `
Array [
Array [
Object {
"overscanStartIndex": 0,
"overscanStopIndex": 6,
"visibleStartIndex": 0,
"visibleStopIndex": 4,
},
],
]
`;
exports[`FixedSizeList scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
exports[`FixedSizeList scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
src/__tests__/__snapshots__/VariableSizeGrid.js.snap
+2
View File
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`VariableSizeGrid scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
exports[`VariableSizeGrid scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
src/__tests__/__snapshots__/VariableSizeList.js.snap
+2
View File
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`VariableSizeList scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
exports[`VariableSizeList scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
src/createGridComponent.js
+23 -2
View File
@@ -221,6 +221,13 @@ export default function createGridComponent({
scrollLeft: number,
scrollTop: number,
}): void {
if (scrollLeft !== undefined) {
scrollLeft = Math.max(0, scrollLeft);
}
if (scrollTop !== undefined) {
scrollTop = Math.max(0, scrollTop);
}
this.setState(prevState => {
if (scrollLeft === undefined) {
scrollLeft = prevState.scrollLeft;
@@ -229,6 +236,13 @@ export default function createGridComponent({
scrollTop = prevState.scrollTop;
}
if (
prevState.scrollLeft === scrollLeft &&
prevState.scrollTop === scrollTop
) {
return null;
}
return {
horizontalScrollDirection:
prevState.scrollLeft < scrollLeft ? 'forward' : 'backward',
@@ -250,10 +264,17 @@ export default function createGridComponent({
columnIndex?: number,
rowIndex?: number,
}): void {
const { height, width } = this.props;
const { columnCount, height, rowCount, width } = this.props;
const { scrollLeft, scrollTop } = this.state;
const scrollbarSize = getScrollbarSize();
if (columnIndex !== undefined) {
columnIndex = Math.max(0, Math.min(columnIndex, columnCount - 1));
}
if (rowIndex !== undefined) {
rowIndex = Math.max(0, Math.min(rowIndex, rowCount - 1));
}
const estimatedTotalHeight = getEstimatedTotalHeight(
this.props,
this._instanceProps
@@ -677,7 +698,7 @@ export default function createGridComponent({
const { direction } = this.props;
// HACK According to the spec, scrollLeft should be negative for RTL aligned elements.
// Chrome does not seem to adhere; its scrolLeft values are positive (measured relative to the left).
// Chrome does not seem to adhere; its scrollLeft values are positive (measured relative to the left).
// See https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollLeft
let calculatedScrollLeft = scrollLeft;
if (direction === 'rtl') {
src/createListComponent.js
+13 -5
View File
@@ -180,19 +180,27 @@ export default function createListComponent({
}
scrollTo(scrollOffset: number): void {
this.setState(
prevState => ({
scrollOffset = Math.max(0, scrollOffset);
this.setState(prevState => {
if (prevState.scrollOffset === scrollOffset) {
return null;
}
return {
scrollDirection:
prevState.scrollOffset < scrollOffset ? 'forward' : 'backward',
scrollOffset: scrollOffset,
scrollUpdateWasRequested: true,
}),
this._resetIsScrollingDebounced
);
};
}, this._resetIsScrollingDebounced);
}
scrollToItem(index: number, align: ScrollToAlign = 'auto'): void {
const { itemCount } = this.props;
const { scrollOffset } = this.state;
index = Math.max(0, Math.min(index, itemCount - 1));
this.scrollTo(
getOffsetForIndexAndAlignment(
this.props,