Ajatt-Tools / anki.koplugin

KOReader plugin enabling Anki card generations for words looked up in the internal dictionary.
59 stars 6 forks source link

KOReader crash on "Add to Anki" #13

Closed paneutral closed 11 months ago

paneutral commented 11 months ago

When "Add to Anki" is pressed or long-pressed on a query via the manual dictionary lookup, KOReader crashes. Not all documents are OCR-able, making this function essential.

The problem seems to be pretty straightforward. It fails to determine that there is no context and attempts to concatenate an uninitialized variable. crash

nairyosangha commented 11 months ago

Pretty easy to reproduce indeed, I hadn't considered this scenario. I don't immediately see anything in the popup dict that indicates whether it was opened from the menu (manual lookup) or by selecting something in a book. However, just checking whether there's any text selected should do the trick: https://github.com/Ajatt-Tools/anki.koplugin/commit/4ecce4afe4ec00d424b914743c961b79cc54dd7c

paneutral commented 11 months ago

thanks, this works for all practical purposes but it'll still crash if you search from the main menu because ui.highlight isn't initialized. i don't know lua, but using this: contextual_lookup = self.ui.highlight ~= nil and self.ui.highlight.selected_text ~= nil, works up until the point that it tries to access ui.document in one of those get functions.

nairyosangha commented 11 months ago

Kind of a strange use case to be honest, the plugin tries to get a bunch of info from the document you're reading, which you can't do now, I feel like it would make more sense to not have the 'add to anki' button show up at all when you're not accessing the dictionary from within a document.

But since it's already half supported now we might as well make sure it works completely https://github.com/Ajatt-Tools/anki.koplugin/commit/b6b4ead806b9f8b51d67408efd4fe4ba78e99905

paneutral commented 11 months ago

yeah it's not a legitimate use case, i just wanted to let you know that the user can crash KOReader. They can still crash it now via get_language() if the dict has no language (which applies to all of mine).

nairyosangha commented 11 months ago

So just to be clear, you'd be okay with the 'Add to Anki' button only being present when the reader is open? So you can't create cards when you don't have a document open. If I understood you correctly, and you ran into an issue when manually looking up a word in a document you were reading where you could not select the text, I think this would be a decent way to fix it

nairyosangha commented 11 months ago

Pushed on a separate branch first: https://github.com/Ajatt-Tools/anki.koplugin/tree/BUG/prevent_koreader_crash

I hope you're not able to make it crash now :smile:

paneutral commented 11 months ago

So just to be clear, you'd be okay with the 'Add to Anki' button only being present when the reader is open?

Exactly, since I'm reading documents which don't work with OCR. I use a gesture shortcut to open the manual dictionary search.

I don't foresee a circumstance where someone thinks of a word on the spot while in the file manager. I only ran into that issue because I was lazy during troubleshooting and didn't open a file to see if your patch works.

This new patch works flawlessly, thank you!

nairyosangha commented 11 months ago

Cool, I'll merge that patch to master then and consider this done, feel free to reopen if I forgot something