Crash on preview of card added to dynamic deck #5756
Comments
|
Step 2 in your reproduction steps seems incorrect to me. It should (and does for me) go to the default deck by default if you try to open the add note dialog with a filtered deck. We don't allow users to add cards to filtered decks. |
|
To me it seems the real bug here is that the previewer is trying to use the currently selected libanki deck instead of the deck selected in the note editor. Probably it's a bug introduced ages ago by me when we removed the guarantee that the deck selected in libanki is the same as the deck selected in the note editor. |
|
okay - so the critical thing here is that we shouldn't be able to add notes to dynamic decks. I'll hunt through things from that perspective. |
|
I had a similar problem, where clicking on a filtered deck crashed. 2020-02-18 02:12:42.153 15759-15759/com.ichi2.anki E/AndroidRuntime: FATAL EXCEPTION: main Note that the only reason why I obtained the last part is because of PR #5762 (the version of the code I'm running is 1c4ca85) So, it seems that the method is looking for the property used for rescheduling revision in a filtered deck's option. The problem is that the filtered deck option is itself, and it has no "rev" value. I just checked, there is no card directly in the filtered deck, so that's not the problem. |
|
This particular bug was introduced in 3a7bea5 by PR #5733 of @correia55. |
|
I'll revert that if we can't figure it out quickly. It's not a huge PR so it should possible to figure it out but crashing is obviously not good |
|
I think I've got this figured out in #5757 - Going to merge it and push it out as a fix and release 2.9.4 |
|
As for Tim's comment - the card addition is actually going to the default deck, which is what should happen, it's just the preview is happening in the "current deck", which is the filtered deck and because of the assumption all decks (including dynamic) have deck rev options in #5733 the previewer crashes. That's unexpected but by not assuming dynamic decks have rev options everything is fine again |
|
I think you intended to keep this open, until we confirm that we're not (temporarily) adding cards to dynamic decks when previewing. |
|
No - I can't see how this is related. I can't even see how the patch that blew up AbstractCardViewer affected it. I think it was latent, so I opened #5765 as a very focused - full-steps-to-reproduce-the-test issue. But this can stay open too now that they're cross-linked |
|
#5765 is a non-issue as far as I can tell... If you think it's necessary to check it then please feel free to re-open. But what I want is an understanding of the root cause of this crash. I have been assuming the card added here was incorrectly added to a dynamic deck. On quick inspection of the code I'm not so sure about that anymore, but then the question is why was this code path crashing? |
|
I'm not exactly sure of course - I thought #5765 would tickle it during inspection. My quick inspection did not lead me to believe it would actually add the card to the deck but proof is needed and it's an opportunity to learn more about the template editor which will serve me well for the mega-patch there anyway. Without saying it's a root cause, all I know for sure is that after adding deck-specific auto-review settings, AbstractCardReviewer could crash if the current deck was dynamic. Why was the CardTemplateEditor pulling up the dynamic deck (selected in deck list) as the thing to preview (causing the crash) vs the deck in the spinner? Unsure. |
|
Most likely the problem is not actually that the dummy card is added to a dynamic deck, but rather AbstractFlashcardViewer is assuming the card being reviewed (in this case a dummy card) is in the currently selected deck (which happens to be a dynamic deck). In fact the dummy card used in the previewer is always in the default deck, not the currently selected deck. So the real underlying problem is that any code that wants to know something about the deck that the card is in, probably needs to go in the |
|
That seems right based on what I know and is easy to confirm, I'll follow that path when I get back if you don't beat me to it |
|
@david-allison-1 your recent commits may have closed this one, or it may be that the hand-off between card editor -> card template editor -> previewer is not selecting the right deck for preview somehow? |
|
@mikehardy Nope, happy to take this on though. |
|
Oh wow, I didn't realise |
|
|
Then just for fun you'll love scanning #5151 😮 |
|
@mikehardy I'll bet you've been wanting me to look at that for a few days now 😉. I've skimmed, but I'm not sure you'll like the answer. My first impressions are:
Practicalities:
All the above is honest thoughts/opinions, you're maintaining the project and the change, and I'll take your lead on solutions/strategies to get to this. |
Previewing a card currently performs displayAnswerBottomBar which requires an ease calculation. The ease calculation depends on the current deck, and is not necessary in the previewer as we don't show ease buttons, we get a crash as the scheduler is pointed at the wrong deck, so don't calculate either. Note: This only fixes the crash, it doesn't touch the root cause: We rely on libAnki to get cards for previews, which relies on the current deck for setting certain data. This information is neither required, nor correct when previewing. We can't change the scheduler while previewing as we may be previewing during a review session, which would cause a scheduler reset.
|
If you're talking about #5151 it was actually split into separate items as you mention prior but guidance at the time was to combine, so it's in the current state based on that. There's actually only one outstanding item left pre-merge for that one, and then I'm going to merge it. It's not the most fantastic but it works well and has nearly 100% coverage (in fact, most of the testing infrastructure was co-developed at the time in order to make sure I didn't bungle it...) |
* NF: Rename showEaseButtons to displayAnswerBottomBar * #5756 - Fix Dynamic Card Preview Crash Previewing a card currently performs displayAnswerBottomBar which requires an ease calculation. The ease calculation depends on the current deck, and is not necessary in the previewer as we don't show ease buttons, we get a crash as the scheduler is pointed at the wrong deck, so don't calculate either. Note: This only fixes the crash, it doesn't touch the root cause: We rely on libAnki to get cards for previews, which relies on the current deck for setting certain data. This information is neither required, nor correct when previewing. We can't change the scheduler while previewing as we may be previewing during a review session, which would cause a scheduler reset.
|
2.10alpha54 released just now has this whenever google delivers it to you |
* NF: Rename showEaseButtons to displayAnswerBottomBar * #5756 - Fix Dynamic Card Preview Crash Previewing a card currently performs displayAnswerBottomBar which requires an ease calculation. The ease calculation depends on the current deck, and is not necessary in the previewer as we don't show ease buttons, we get a crash as the scheduler is pointed at the wrong deck, so don't calculate either. Note: This only fixes the crash, it doesn't touch the root cause: We rely on libAnki to get cards for previews, which relies on the current deck for setting certain data. This information is neither required, nor correct when previewing. We can't change the scheduler while previewing as we may be previewing during a review session, which would cause a scheduler reset.
Saw our first google auto-test-lab-on-new-release crash report in a long time
https://play.google.com/apps/publish/?account=5338425030304417978&pli=1#PreLaunchReportPlace:p=com.ichi2.anki&plrtab=CRASH&plrvc=21000136
Reproduction Steps
Expected Result
You should see a preview
Actual Result
So here's the questions:
Debug info
Refer to the support page if you are unsure where to get the "debug info".
Research
Enter an [ x ] character to confirm the points below:
[ ] I have read the support page and am reporting a bug or enhancement request specific to AnkiDroid
[ ] I have checked the manual and the FAQ and could not find a solution to my issue
[ ] I have searched for similar existing issues here and on the user forum
The text was updated successfully, but these errors were encountered: