Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash in AbstractFlashCardViewer.onCollectionLoaded() ArrayIndexOutOfBoundsException #4987

Closed
mikehardy opened this issue Sep 20, 2018 · 6 comments · Fixed by #4988
Closed

Crash in AbstractFlashCardViewer.onCollectionLoaded() ArrayIndexOutOfBoundsException #4987

mikehardy opened this issue Sep 20, 2018 · 6 comments · Fixed by #4988
Assignees
@mikehardy
Labels
Accepted Maintainers welcome a PR implementing this feature Bug Priority-Medium

Comments

@mikehardy
Copy link
Member

The lifecycle changes in #4972 are the gift that keep on giving :-)

Reproduction Steps
  1. Click top-left hamburger to get NavDrawer, Select Card Browser
  2. Either have an empty collection or search for something that doesn't exist so the browser is empty
  3. Click top-right options menu, note "Preview" option exists, tap it
Expected Results

This menu is already dynamic, so probably shouldn't have the Preview option there when there are no cards, but if it is there, it should toast "No cards to preview" or something.

Actual Result

Issue 1 - Crash when you hit "Preview" on the options menu in card browser when there are no cards.

Issue 2 - Also, the Card Browser Preview cycling is inconsistent. It starts with a cycle-left button but if you cycle right then back all the way left, the button is disabled. Should be disabled to start.

Issue 3 - Also the cycling right vs left to advance is probably a localization RTL issue (ha!)

This is from the pre-launch Firebase "Robo" test that Google Play conducts automatically on all releases, with 2.9alpha36

The "MATE 9" device here https://play.google.com/apps/publish/?account=5338425030304417978#PreLaunchReportPlace:p=com.ichi2.anki&appid=4973711737547064258&plrtab=CRASH&plrvc=20900136

09-20 07:16:50.650: E/MonitoringInstr(9942): Exception encountered by: com.ichi2.anki.Previewer@5936a01. Dumping thread state to outputs and pining for the fjords.
09-20 07:16:50.650: E/MonitoringInstr(9942): java.lang.ArrayIndexOutOfBoundsException: length=0; index=0
09-20 07:16:50.650: E/MonitoringInstr(9942): at com.ichi2.anki.Previewer.onCollectionLoaded(Previewer.java:57)
09-20 07:16:50.650: E/MonitoringInstr(9942): at com.ichi2.anki.AnkiActivity.startLoadingCollection(AnkiActivity.java:253)
09-20 07:16:50.650: E/MonitoringInstr(9942): at com.ichi2.anki.AbstractFlashcardViewer.onCreate(AbstractFlashcardViewer.java:826)
09-20 07:16:50.650: E/MonitoringInstr(9942): at com.ichi2.anki.Previewer.onCreate(Previewer.java:48)
09-20 07:16:50.650: E/MonitoringInstr(9942): at android.app.Activity.performCreate(Activity.java:6910)
09-20 07:16:50.650: E/MonitoringInstr(9942): at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1123)
09-20 07:16:50.650: E/MonitoringInstr(9942): at androidx.test.runner.MonitoringInstrumentation.callActivityOnCreate(MonitoringInstrumentation.java:215)
@mikehardy mikehardy added Bug Priority-Medium Accepted Maintainers welcome a PR implementing this feature labels Sep 20, 2018
@mikehardy mikehardy self-assigned this Sep 20, 2018
@mikehardy
Copy link
Member Author

I already have a fix for item 1 - it's trivial (famous last words)
I'll have a fix for 2 and maybe 3 (assuming it's makes sense and isn't a nightmare) in a bit

@mikehardy
Copy link
Member Author

mikehardy commented Sep 20, 2018 •
edited
Loading

Previewer also loses CardBrowser search state when it returns, it's a nest of little bugs up in there

mikehardy added a commit to mikehardy/Anki-Android that referenced this issue Sep 20, 2018
@mikehardy
Avoid collection load if finishing, Fixes ankidroid#4987
96ed621
@mikehardy
Copy link
Member Author

Video that shows:
1- Previewer with 3 cards starting with wrong left-button in cycle, and after going all the way forward/right (6 clicks), as you go back, you skip some steps and end up with 4 clicks.
2- Previewer with an empty deck still shows the Preview button, then crashes

Click this for youtube:
Busted Preview Menu and Cycling

Second video that shows:
1- Previewer on an empty deck doesn't show Preview (Previewer itself is also hardened against an Intent that calls it with a zero deck, but can't show that)
2- Previewer with 3 cards starts with disabled left/previous button, and takes 6 steps to cycle through forward, and 6 clicks to go through backward, seeing all display steps each direction

Click this for youtube:
Busted Preview Menu and Cycling

@mikehardy
Copy link
Member Author

First video shows the problem:

  1. preview cycling starts with enabled previous button, and
  2. after clicking 6 times forward to end, only click 4 times back? skips one of the states on each card
  3. preview option shows up on a menu for empty browser, and crashes

Click here for youtube:
Broken Previewer

Need to post the PR but this is the after video:

  1. preview option doesn't show on empty deck
  2. cycling starts with disabled previous button
  3. cycling does not skip steps, 6 clicks forward, 6 clicks back - all states shown

click here for youtube:
Fixed Previewer

@timrae
Copy link
Member

timrae commented Sep 21, 2018

I don't think this crash is related to the loader changes, it was probably introduced in the multi-select work

@mikehardy
Copy link
Member Author

Maybe I did not check that far since reproduction was so easy. Almost done with search term preservation in instance state and I'll PR the batch

@mikehardy mikehardy mentioned this issue Sep 21, 2018
Merged
5 tasks
mikehardy added a commit to mikehardy/Anki-Android that referenced this issue Sep 27, 2018
@mikehardy
Avoid loading collection if finishing() == true
646be33 
This was proposed as a fix for ankidroid#4987 but that issue is being fixed
in a defense-in-depth fashion as this is a threading change and
harder to reason about
@mikehardy mikehardy mentioned this issue Sep 27, 2018
Merged
mikehardy added a commit to mikehardy/Anki-Android that referenced this issue Oct 1, 2018
@mikehardy
Hide Preview option if no cards, remember search
f807afa 
- Previously Preview was always available, fixes ankidroid#4987 crash when no cards
- Previously if you went to another Activity with an active search, it was forgotten
mikehardy added a commit that referenced this issue Oct 2, 2018
@mikehardy
Hide Preview option if no cards, remember search
d035d8d 
- Previously Preview was always available, fixes #4987 crash when no cards
- Previously if you went to another Activity with an active search, it was forgotten
mikehardy added a commit to mikehardy/Anki-Android that referenced this issue Oct 15, 2018
@mikehardy
Avoid loading collections when not needed
18601e6 
Collection loading logic was a bit disjointed between super- and
sub-class, this puts it one place, and makes sure we won't load
the collection unless we're going to actually create the activity

This is the final part that Fixes ankidroid#4987
timrae pushed a commit that referenced this issue Oct 16, 2018
@mikehardy @timrae
Avoid loading collections when not needed
aa26dbe 
Collection loading logic was a bit disjointed between super- and
sub-class, this puts it one place, and makes sure we won't load
the collection unless we're going to actually create the activity

This is the final part that Fixes #4987
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mikehardy mikehardy

Labels
Accepted Maintainers welcome a PR implementing this feature Bug Priority-Medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue4987/previewer issues mikehardy/Anki-Android
2 participants
@mikehardy @timrae