(Selection) Unrecycled detached ViewHolders can't be updated when using StableIdKeyProvider. [145767095]

Fixed

Bug

Status Update

No update yet.

Description

na...@gmail.com

created issue #1

Dec 6, 2019 01:33AM

The scenario:

- I have a RecyclerView using Selection with SelectionPredicates.createSelectSingleAnything() and StableIdKeyProvider.
- A selected item will be colored RED, unselected item will be colored GREEN.
- I select item whose adapterPosition is 0.
- I scroll the list so the item whose adapterPosition is 1 will be on top.
--> What I actually want is the item at '0' will be offscreen and will be detached, but not enough for recycle.
- I select item of adapterPosition '1'.
--> Because it is single selection list, I expect that the item of position '0' will *appear to be unselected*.
- I scroll to top and see if item of position '0' appears to be unselected or not.

Expect: item of position 0 will be colored GREEN (it should appear to be unselected), item of position 1 will be colored RED (it should appear to be selected).

Actual: item of position 0 is colored RED (it appears to be selected), item of position 1 is colored RED (it appears to be selected). (See attachment: recyclerview_selection_issue.mp4).

Note that, the item is correctly unselected, but the change is never delivered to the Adapter due to the behavior of StableIdKeyProvider (I have post about this here:

https://twitter.com/ene__im/status/1202477885505691648), so it cannot "appear" to be unselected.

Source code:

https://github.com/eneim/RecyclerView-Selection-Bug

Version used (can be found in the repo): RecyclerView 1.1.0, Selection 1.1.0-beta01.

Thanks in advance.

recyclerview_selection_issue.mp4

4.2 MB

Download

Comments

em...@google.com <em...@google.com> May 4, 2020 10:38PM

Assigned to ry...@google.com.

ry...@google.com <ry...@google.com> May 6, 2020 06:21AM

Reassigned to sm...@google.com.

sm...@google.com <sm...@google.com> #2Jul 27, 2020 07:19PM

Project: platform/frameworks/support
Branch: androidx-main

commit 2099e20d0af61318ec59b5c6391498304979e024
Author: Clara Fok <clarafok@google.com>
Date: Wed May 03 14:38:14 2023

Migrate paging-testing to PagingSourceFactory

Migrate existing usages of lambda type () -> PagingSource<Key, Value> to concrete PagingSourceFactory<Key, Value> type

Test: ./gradlew paging:paging-testing:cC
Bug: 280655188
Relnote: "Migrated paging-testing's use of lambda type () -> PagingSource<Key, Value> to type PagingSourceFactory<Key, Value>."
Change-Id: I4a95067989fabf8258a3392007dbf8abd35d8efa

M paging/paging-testing/api/current.txt
M paging/paging-testing/api/public_plus_experimental_current.txt
M paging/paging-testing/api/restricted_current.txt
M paging/paging-testing/src/main/java/androidx/paging/testing/StaticListPagingSourceFactory.kt
M paging/paging-testing/src/test/kotlin/androidx/paging/testing/PagerFlowSnapshotTest.kt
M paging/paging-testing/src/test/kotlin/androidx/paging/testing/StaticListPagingSourceFactoryTest.kt

https://android-review.googlesource.com/2579311

na...@gmail.com <na...@gmail.com> #3Jul 28, 2020 12:22AM

Project: platform/frameworks/support
Branch: androidx-main

commit f06753ed29d67623b4061cb222717948d0a37ef7
Author: Clara Fok <clarafok@google.com>
Date: Wed May 03 13:16:53 2023

Add PagingSourceFactory interface

This functional interface allows creating a PagingSourceFactory by passing in a lambda that returns a PagingSource, or by creating a class that extends this interface. The factory can be passed into Pager as the pagingSourceFactory.

Test: ./gradlew paging:paging-common:test
Bug: 280655188
Relnote: "Added a new PagingSourceFactory functional interface. Override the invoke() method with an implementation that returns a new instance of PagingSource. This factory can be used to instantiate a Pager."
Change-Id: I3316590a0d667566edeb40d4e99e005cd6cee3ac

M paging/paging-common/api/current.txt
M paging/paging-common/api/public_plus_experimental_current.txt
M paging/paging-common/api/restricted_current.txt
M paging/paging-common/src/main/kotlin/androidx/paging/InvalidatingPagingSourceFactory.kt
A paging/paging-common/src/main/kotlin/androidx/paging/PagingSourceFactory.kt

https://android-review.googlesource.com/2579310

sm...@google.com <sm...@google.com> #4Jul 28, 2020 04:24PM

Project: platform/frameworks/support
Branch: androidx-main

commit 967c97f18d7f2a717222b09be6227f0f7dabb323
Author: Clara Fok <clarafok@google.com>
Date: Thu May 04 13:06:24 2023

Update documentation for asPagingSourceFactory API

Kdocs updated to reflec the API's migration from returning a lambda to returning a Class.

Test: n/a
Bug: 280655188
Change-Id: Ib3f9b5c9215ba9e5ee38d6b192e9858bbe080346

M paging/paging-testing/src/main/java/androidx/paging/testing/StaticListPagingSourceFactory.kt

https://android-review.googlesource.com/2580094

ap...@google.com <ap...@google.com> #5Jul 31, 2020 09:32PM

Project: platform/frameworks/support
Branch: androidx-main

commit 59e0f757ba85b81b800e6fb0d60dd25dd88bee54
Author: Clara Fok <clarafok@google.com>
Date: Thu May 04 12:52:34 2023

Expand asPagingSourceFactory API to singe list

In addition to Flow<List<Value>>.asPagingSourceFactory API, we added a List<Value>.asPagingSourceFactory. All PagingSoruces produced by this new API will load from the exact same static list of data. It supports multi-generational operations such as refresh, but it does not support any updates to the source data. Because it operates on a static list, collection is not required which removes the requirement for a coroutineScope.

Test: ./gradlew paging:paging-testing:test
Bug: 280655188
Relnote: "Added new API List<Value>.asPagingSourceFactory to get a factory that creates PagingSource instances that loads from the immutable list of data."
Change-Id: Id34d1361119413d0c19a054a2e23dd26241faa6f

M paging/paging-testing/api/current.txt
M paging/paging-testing/api/public_plus_experimental_current.txt
M paging/paging-testing/api/restricted_current.txt
M paging/paging-testing/src/main/java/androidx/paging/testing/StaticListPagingSourceFactory.kt
M paging/paging-testing/src/test/kotlin/androidx/paging/testing/PagerFlowSnapshotTest.kt
M paging/paging-testing/src/test/kotlin/androidx/paging/testing/StaticListPagingSourceFactoryTest.kt

https://android-review.googlesource.com/2580093

sm...@google.com <sm...@google.com> #6Aug 3, 2020 08:42PM

Fixed internally and will be available in paging 3.2.0-alpha06.

ap...@google.com <ap...@google.com> #7Aug 5, 2020 06:35PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.paging:paging-common:3.2.0-alpha06
androidx.paging:paging-testing:3.2.0-alpha06

ap...@google.com <ap...@google.com> #8Aug 5, 2020 06:35PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ce1b333030414b89b1980fa2327774af1b5416dc
Author: Steve McKay <smckay@google.com>
Date: Thu Jul 30 09:33:41 2020

Add support for multiple RecyclerListeners.

This feature is necessitated by RecyclerView-Selection dependency on listening
to recycle events.

Bug: 162504379
Test: Added new coverage.
Relnote: Add support for multiple RecyclerListeners as necessitated to fix

b/145767095

affecting RecyclerView-Selection.
Change-Id: I70ad8f9bcf25c2c00fbf5f71d5a991287bef1606

M leanback/leanback/api/current.txt
M leanback/leanback/api/public_plus_experimental_current.txt
M leanback/leanback/api/restricted_current.txt
M recyclerview/recyclerview/api/current.txt
M recyclerview/recyclerview/api/public_plus_experimental_current.txt
M recyclerview/recyclerview/api/restricted_current.txt
A recyclerview/recyclerview/src/androidTest/java/androidx/recyclerview/widget/RecyclerListenerTest.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/RecyclerView.java

https://android-review.googlesource.com/1381078

sm...@google.com <sm...@google.com> #9Apr 22, 2021 09:24PM

Planning on the fix being released in 1.2. Should be available in 1.2.0-alpha01 in the next few weeks. No telling for sure how long it will take to get 1.2.0 to stable.

cr...@gmail.com <cr...@gmail.com> #10May 22, 2024 09:42PM

With the 1.2.0-alpha01 it seems there's a new issue now, caused by the changes, that results in the holder at the top of StableIdKeyProvider.onRecycled() to always be null. This still results in the same issue that the cache isn't updated properly and the that can cause crashes, because the lookup is wrong.