Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Using stable IDs in TabsAdapter crashes when deleting first tab #18668

Closed
jonalmeida opened this issue Mar 26, 2021 · 2 comments
Closed

Using stable IDs in TabsAdapter crashes when deleting first tab #18668

jonalmeida opened this issue Mar 26, 2021 · 2 comments
Assignees
Labels
E3 Estimation Point: average, 2 - 3 days Feature:SyncTabs Sync tabs Feature:Tabs MR1 Issues that are needed for the MR1 2021 release.
Milestone

Comments

@jonalmeida
Copy link
Contributor

jonalmeida commented Mar 26, 2021

Add the following the TabsAdapter in browser-tabstray:

init {
    setHasStableIds(true)
}

override fun getItemId(position: Int): Long {
    return position.toLong()
}
2021-03-26 13:32:52.044 32127-32127/org.mozilla.samples.browser E/ExceptionHandler: Uncaught exception handled: 
    java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid item position 0(offset:-1).state:3 androidx.recyclerview.widget.RecyclerView{2902363 VFED..... ......ID 0,154-1080,2050 #7f09021a app:id/tabsTray}, adapter:mozilla.components.browser.tabstray.TabsAdapter@e038680, layout:androidx.recyclerview.widget.GridLayoutManager@17b74b9, context:org.mozilla.samples.browser.BrowserActivity@2b2f2c1
        at androidx.recyclerview.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:6183)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:6118)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:6114)
        at androidx.recyclerview.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2303)
        at androidx.recyclerview.widget.GridLayoutManager.layoutChunk(GridLayoutManager.java:561)
        at androidx.recyclerview.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1587)
        at androidx.recyclerview.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:675)
        at androidx.recyclerview.widget.GridLayoutManager.onLayoutChildren(GridLayoutManager.java:170)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep1(RecyclerView.java:4085)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3849)
        at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4404)

┆Issue is synchronized with this Jira Task

@jonalmeida jonalmeida self-assigned this Mar 26, 2021
@jonalmeida
Copy link
Contributor Author

We know that adding stable IDs and getItemId for multi-select is needed and indeed correct as we can see it in the demo sunflower app as well.

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Mar 26, 2021

Paired on this with @eliserichards and went through the TabsPresenter code to verify we were doing everything right (we were).

The issue turned out to be our reliance on the sunflower app's implementation of getItemId:

override fun getItemId(position: Int): Long = position.toLong()

On closer inspection, we remembered that we already solved this problem for the AwesomeBar which needed unique namespaced IDs for each suggestion.

We can follow the same strategy to solve the problem in the TabsAdapter. 🎉

@jonalmeida jonalmeida transferred this issue from mozilla-mobile/android-components Mar 27, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Mar 27, 2021
@jonalmeida jonalmeida added Feature:SyncTabs Sync tabs Feature:Tabs MR1 Issues that are needed for the MR1 2021 release. and removed needs:triage Issue needs triage labels Mar 27, 2021
@jonalmeida jonalmeida moved this from Ready for Engineering (min-5 ; max-22) to Review in progress (WIP limit - 11) in Android Engineering Team Kanban board Mar 27, 2021
@jonalmeida jonalmeida added this to In progress in Tabs Tray Refactor Mar 29, 2021
@jonalmeida jonalmeida added the E3 Estimation Point: average, 2 - 3 days label Mar 29, 2021
Android Engineering Team Kanban board automation moved this from Review in progress (WIP limit - 11) to Done Mar 29, 2021
Tabs Tray Refactor automation moved this from In progress to Done Mar 29, 2021
@gabrielluong gabrielluong added this to the 89 milestone Mar 30, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E3 Estimation Point: average, 2 - 3 days Feature:SyncTabs Sync tabs Feature:Tabs MR1 Issues that are needed for the MR1 2021 release.
Projects
No open projects
Development

No branches or pull requests

2 participants