epoxy icon indicating copy to clipboard operation
epoxy copied to clipboard

Sticky headers displays wrong header or crashes after dataset change

Open cbeyls opened this issue 2 years ago • 12 comments

StickyHeaderLinearLayoutManager does not handle adapter dataset changes correctly.

Specifically, it doesn't handle properly the case where a set of changes notifications includes an insert followed by one or more deletes or moves. This causes the wrong view to be used as sticky header in some cases, and in rare cases it can also cause an out of bounds error because the code tries to access data outside the range of the adapter.

The flawed logic is located in the child class HeaderPositionsAdapterDataObserver. As long as items are deleted or moved, everything works fine. But when onItemRangeInserted() is called, there is a problem: the code tries to access the adapter immediately to check if there are headers in the new inserted elements, but instead it must wait for the other adapter updates to be dispatched before accessing the adapter data to be able to compute the adapter positions correctly.

Because adapter updates are asynchronous, multiple calls to notify* can be done in a row for a single change of adapter data. It's only after that, during the next layout pass that the adapter data can be accessed.

Here is a simple example to illustrate the issue:

  • An adapter initially contains 4 items
  • The items list is updated with a new list of 4 items: a new item has been added at the end and the first item has been deleted
  • To notify the changes, DiffUtil will call notifyItemInserted(4, 1) immediately followed by notifyItemDeleted(0, 1)
  • When onItemRangeInserted() is called, HeaderPositionsAdapterDataObserver in StickyHeaderLinearLayoutManager will immediately try to access the adapter element at position 4 to check for new headers. But this position is out of range: the list only contains 4 elements. The adapter needs to wait for the following call to onItemRangeRemoved() to be able to compute the adapter positions correctly. The adapter should only be accessed asynchronously during the next layout pass.

Because of this, sticky headers are unstable and cannot be used with DiffUtil. Only notifyDataSetChanged() works reliably.

cbeyls avatar Jul 17 '21 11:07 cbeyls