Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

[Bug] NPE in ExpandableLayout when sticky item is the last item and items list changes #10108

Closed
Mugurell opened this issue Apr 19, 2021 · 0 comments · Fixed by #10111
Closed

[Bug] NPE in ExpandableLayout when sticky item is the last item and items list changes #10108

Mugurell opened this issue Apr 19, 2021 · 0 comments · Fixed by #10111
Assignees
@Mugurell
Labels
<menu> Component: concept-menu, browser-menu, browser-menu2
Projects
Android Engineering Team Kanban board Three-dot menu redesign
Milestone
75.0.0 🍇

Comments

@Mugurell
Copy link
Contributor

Mugurell commented Apr 19, 2021 •
edited by data-sync-user 

java.lang.NullPointerException: listView.getChildAt(stickyItemIndex) must not be null
    at mozilla.components.browser.menu.view.ExpandableLayout.calculateCollapsedHeight$browser_menu_release(ExpandableLayout.kt:358)
    at mozilla.components.browser.menu.view.ExpandableLayout.getOrCalculateCollapsedHeight$browser_menu_release(ExpandableLayout.kt:233)
    at mozilla.components.browser.menu.view.ExpandableLayout.onMeasure(ExpandableLayout.kt:119)
    at android.view.View.measure(View.java:25466)

This would be a blocker for Fenix using a sticky footer (mozilla-mobile/fenix#19018 (comment)) it being an edgecase seen in the following scenario:

  • the sticky footer is the last item in the adapter
  • elements laid out differ from the ones in adapter (this happens in Fenix with uses for example a WebExtensionPlaceholderMenuItem)

┆Issue is synchronized with this Jira Task
@Mugurell Mugurell self-assigned this Apr 19, 2021
Mugurell added a commit to Mugurell/android-components that referenced this issue Apr 19, 2021
@Mugurell
For mozilla-mobile#10108 - Reconcile list and adapter positions in Ex…
9a5e2fb 
…pandableLayout

ExpandableLayout uses adapter based indexes to identify children.
In dynamic menus with list items being hidden this could lead to invalid
calculations and even to exceptions for indexes bigger that the then current
children count in the list.

To reconcile this we'll calculate a valid lastVisibleItemIndexWhenCollapsed and
stickyItemIndex to be used before the operations in calculateCollapsedHeight().

Because the code is now clearly tightly coupled to RecyclerView this patch also
refactors a previous general ViewGroup type to RecyclerView and because of this
part of the previous tests needed some adjustments.
@eliserichards eliserichards linked a pull request Apr 19, 2021 that will close this issue
For #10108 - Reconcile list and adapter positions in ExpandableLayout #10111
Merged
4 tasks
This was referenced Apr 19, 2021
Merged
Merged
@eliserichards eliserichards added this to Ready for Engineering (min-5 ; max-22) in Android Engineering Team Kanban board via automation Apr 19, 2021
@eliserichards eliserichards moved this from Ready for Engineering (min-5 ; max-22) to Review in progress (WIP limit - 11) in Android Engineering Team Kanban board Apr 19, 2021
@eliserichards eliserichards added this to In progress in Three-dot menu redesign Apr 19, 2021
@eliserichards eliserichards added the <menu> Component: concept-menu, browser-menu, browser-menu2 label Apr 19, 2021
@mergify mergify bot closed this as completed in #10111 Apr 19, 2021
Android Engineering Team Kanban board automation moved this from Review in progress (WIP limit - 11) to Done Apr 19, 2021
mergify bot added a commit that referenced this issue Apr 19, 2021
@Mugurell @mergify
For #10108 - Reconcile list and adapter positions in ExpandableLayout (…
72d7370 
…#10111)

ExpandableLayout uses adapter based indexes to identify children.
In dynamic menus with list items being hidden this could lead to invalid
calculations and even to exceptions for indexes bigger that the then current
children count in the list.

To reconcile this we'll calculate a valid lastVisibleItemIndexWhenCollapsed and
stickyItemIndex to be used before the operations in calculateCollapsedHeight().

Because the code is now clearly tightly coupled to RecyclerView this patch also
refactors a previous general ViewGroup type to RecyclerView and because of this
part of the previous tests needed some adjustments.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@gabrielluong gabrielluong added this to the 75.0.0 milestone Apr 20, 2021
@eliserichards eliserichards moved this from In progress to Done in Three-dot menu redesign Apr 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Mugurell Mugurell

Labels
<menu> Component: concept-menu, browser-menu, browser-menu2
Projects
No open projects
Android Engineering Team Kanban board
Done
Three-dot menu redesign
Done
Milestone
75.0.0 🍇
Development

Successfully merging a pull request may close this issue.

For #10108 - Reconcile list and adapter positions in ExpandableLayout Mugurell/android-components
3 participants
@gabrielluong @Mugurell @eliserichards