Anki-android: When I refuse recording permission, the application gets stuck.

Created on 16 Aug 2019  路  14Comments  路  Source: ankidroid/Anki-Android

Reproduction Steps
  1. Click on the menu
  2. Click "card browser"
  3. Click the plus sign in the lower right corner
  4. Click the plus sign in the upper right corner
  5. Click the "Add Attachments" button
  6. Click "record audio"
  7. Deny permission requests and request no longer to be displayed
Expected Result

Just can't perform the recording function.

Actual Result

Interface stuck

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:

[ x ] I have read the support page and am reporting a bug or enhancement request specific to AnkiDroid

[ x ] I have checked the manual and the FAQ and could not find a solution to my issue

[ x ] I have searched for similar existing issues here and on the user forum

Latest from F-DROID, 2.8.4, EMUI 9.1.1, HUAWEI nova 5 pro

Recorded video see attachment

anki.zip

All 14 comments

Hello锛宎ny update about this issue锛烮t would be really appreciated to get your concern on this.

Thanks for your amazing app, I also encountered this issue, any plan to fix this?

Hi guys - sorry, we're stretched pretty thin for developer time and while this definitely seems like a bug, it also seems like it falls in the general category of "well, don't do that then..." :-). At the same time, have you tried to reproduce it on the beta to see if it is still valid? The beta will be released as official 2.9 as soon as we finish the docs I think, the code has been ready for a while - https://docs.ankidroid.org/manual.html#betaTesting

Hi, thanks for your reply. I've tried it on the beta 2.9 and the problem still exists. For me, sometimes I refuse some permissions. If it has to be accepted or I get stuck, I will feel unfriendly. If you have time, I hope it can be simply repaired.

Hello, @mikehardy, thanks for your reply, and same old issue here.

Plenty of users may interact with apps pretty fast sometimes, I mean, they could click some buttons very quickly without a doubt or even before they realize what exactly function this button represents. When it comes to granting runtime permissions, perhaps many users prefer to deny them especially it is related to privacy. Well, in this issue, users are sure to agree to grant it since they are using the record function. But chances are excellent that they could accidentally deny the permission request, then it would be really unfriendly and troublesome to get stuck here. So it would be appreciated to get your concern on this and the fix might not cost much effort.

Anyway, thanks a lot for building this useful app!

I'd love to see if fixed and I agree with you completely. No app should just hang for any reason, much less permission denial (which is possible as you say for real reasons and also just accidents). I just haven't had the time to fix it and no one else has either.

Cause
  1. Permission check exits recreateEditingUi() early and performs an async permission check.

https://github.com/ankidroid/Anki-Android/blob/fac394c4a16ee798b0051831ba36bc0cf3a77671/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivity.java#L123-L127

  1. After an async permission check, recreateEditingUi() is called, but does not check whether the permission was granted. On failure (especially with "Deny & Remember" checked), this will infinitely loop.

https://github.com/ankidroid/Anki-Android/blob/fac394c4a16ee798b0051831ba36bc0cf3a77671/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivity.java#L295-L299

Additional Issues
  • This screen is not activity-based. Menu items can invalidate and change the UI (hence the need for recreateEditingUi).
  • No current way of backing out from a UI change if the new screen fails
  • IFieldController requires additional setters to be called for it to be in a valid state.

    • We can currently have a state where a new IFieldController is created, but in an invalid state. Activity.onDestroy() then crashes.

  • "Record Audio" is used as an audio player for the pronunciation feature, this should work, even if audio recording is disabled

I'll get a PR in if the below design seems sensible.


Define 3 transitions (between Multimedia UIs):

  • Init - initial load
  • UI - Menu Item is clicked, (changing from "Record Audio" to "Insert Image").

    • Treat this as cancellable

  • Activity - Pronunciation calls "Record Audio" with a known clip to allow for preview and pronunciation recreation.

    • This is invoked by the caller, so there's no way to know whether the previous UI is in a valid state. Treat this is non-cancellable.


  • Allow "Record Audio" to fail a transition

    • UI - Failing will keep the application on the previous multimedia UI.

    • Requires changes to avoid destroying the previous context until the new context validates.

    • Init - Cancel the Activity

    • Activity - Allow access to "Record Audio" without permissions

  • Save the state of a transition to allow the async onRequestPermissionsResult to reapply the context.
  • Mark the state as having "permissions requested", so they're not re-requested when calling recreateEditingUi
  • Show a toast notification on "Permission Denied"
  • Ensure "Record Audio" can be used without permissions.

@timrae the advanced editor has been a thorn for a while, yes? @david-allison-1 seems interested in (finally!) un-tangling it. If I recall you had some ideas about it, maybe just re-organizing / simplifying the whole thing to be drop-down entries on the paperclip in the note editor vs the current state-based thing?

If we kept the current setup, the design above seems fine as a way to make it function correctly though

Independently - David, thanks so much for pitching in here on some unglamorous items! I really appreciate it

I found @timrae most recent comments and the TL;DR I got from it was "what if we threw away the whole advanced editor thing and just added the items directly to the paperclip menu? https://github.com/ankidroid/Anki-Android/pull/4756#issuecomment-352628301

That PR was one I was just carrying over the finish line from someone else so I didn't want to do such a big change at the time, but the idea (chuck the complicated advanced editor, go with very simple items direct from paperclip) has merit. If you wanted to do that I think the final implementation might be much smaller but it'd be an "overhaul" scale PR, so I could see going either way. If you wanted to keep the existing code (also an idea that has merit - it's complicated but battle tested) then I'd say go with your thoughts above.

Either would be merge-able

I'll go with keeping the code for now, as I've got a working local branch.

I wrote (and lost) a long post on this a while back. My opinion on the matter has changed since then, I think I've come around to the fact that the editor framework provides a lot of debt and little/no value. It's probably best to get rid of it in the long run, but that'll be a large change, and I'd rather not make that under the guise of a crash fix.

I'll go with keeping the code for now, as I've got a working local branch.

I wrote (and lost) a long post on this a while back. My opinion on the matter has changed since then, I think I've come around to the fact that the editor framework provides a lot of debt and little/no value. It's probably best to get rid of it in the long run, but that'll be a large change, and I'd rather not make that under the guise of a crash fix.

Exactly where I was with it as well, that works fine for me

2.10alpha54 released just now has this whenever google delivers it to you

Thank you for the fix! It's greatly appreciated :)

Was this page helpful?
0 / 5 - 0 ratings