yukuku / androidbible

Open Source Bible for Android
https://alkitab.app
Apache License 2.0
311 stars 174 forks source link

Footnotes and Crossreferences are not working in user books with book id greater than 127 #102

Closed restai closed 4 months ago

restai commented 4 months ago

Bug Description

I wanted to create a yes bible module that includes a glossar. So I created a yet file and added the additional content of the glossar to a custom book using book id 201 (as recommended in .yet file format specification):

If the book you want to add is not listed there, you can use any other number starting from 201 up to 255.

Then I added footnotes and crossreferences to the verses of this custom book. After converting the yet file to a yes bible module with YetToYes2.jar I added the bible module to the Quick Bibel App v4.9.0. I f I tap on a footnote or cross reference in the custom book, the footnote or cross reference is sometime shown sometimes an error message appears (Something like 'The footnote with arif cannot be retrieved'). It seems like random, if a footnote or crossreference can be retrieved sucessfully.

Current Behavior

Footnotes and crossreferences in a custom book are randomly working or not.

Expected Behavior

Footnotes and crossreferences in a custom book are always working.

Detailed Technical Bug Description

I investigated the behaviour and found the source of the bug. If a yet file is converted to a yes file, the book, chapter, verse, index values in a footnote/xref line are converted to an arif. These arifs are written to the yes file to the 'footnotes' / 'xrefs' section into the arifs[] array: https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/AlkitabYes2/src/main/java/yuku/alkitab/yes2/section/FootnotesSection.java#L13-L20 https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/AlkitabYes2/src/main/java/yuku/alkitab/yes2/section/XrefsSection.java#L13-L20

The arif array is used to find a footnote/crossreference. This is done using binary search: https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/AlkitabYes2/src/main/java/yuku/alkitab/yes2/section/FootnotesSection.java#L59 https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/AlkitabYes2/src/main/java/yuku/alkitab/yes2/section/XrefsSection.java#L59

And there is the problem: The array is required to be sorted for the binary search (see Android documentation for Arrays.binarySearch(). You mentioned this requirement too). But the array is not sorted, if a book id greater than 127 is used. The order of the arifs in the arifs array of the yes2 file section is in the same order they appear in the yet file (ascending by book, chapter, verse, index; this is enforced by the converter YetToYes2.jar). For book id greater than 127 (128 in the yet file) it leads to an unsorted array because the most significant bit of the arif is 1 so the arif become negative. Example:

xref     40    1    2    1  => arif =   671154689
xref    127    5    2    1  => arif =  2131034625
xref    128    1    1    1  => arif = -2147417855
xref    128    1    1    2  => arif = -2147417854 

Possible Solution

The arif array (and therefore the offset array) should be sorted before written to yes2 file. Most easy solution: Sort the LinkedHashMap or the .entrySet() ascending by key (=arif) before writing the arrays. https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/tools/AlkitabConverter/src/main/java/yuku/alkitabconverter/util/FootnoteDb.java#L102-L122 https://github.com/yukuku/androidbible/blob/eb4462f7e7b81c6415f9704ddf509b74d968fcbb/tools/AlkitabConverter/src/main/java/yuku/alkitabconverter/util/XrefDb.java#L131-L151

At least the recommendation for custom book id/number in the Alkitab / Quick Bible Developer Guide: Bible Version Data (.yet file format specification) should be changed.

yukuku commented 4 months ago

Thank you, @restai for the thorough investigation and suggestions.

I did the fix on the binarysearch function of the app itself, because there could be others who have created the YES file with the sorted-as-unsigned-int arrays of arif.

restai commented 4 months ago

Thank you @yukuku for the fast fix. Indeed it makes more sense to fix it on application side. I hope a new app release is coming soon. Unfortunately the Quick Bible app is still version 4.9.0 and not latest version 4.10.0, the version of the indonesian version Alkitab. In particular the fix for the annoying bug "Footnote dialog links are not clickable" 13a872e6b77b0f4267f589c91c7e789091003243 already in 4.10.0 is not available to Quick Bible app users.

yukuku commented 2 months ago

Thank you @yukuku for the fast fix. Indeed it makes more sense to fix it on application side. I hope a new app release is coming soon. Unfortunately the Quick Bible app is still version 4.9.0 and not latest version 4.10.0, the version of the indonesian version Alkitab. In particular the fix for the annoying bug "Footnote dialog links are not clickable" 13a872e already in 4.10.0 is not available to Quick Bible app users.

Hi @restai, I'm sorry it took so long to update to the newest version. Now, both apps have been updated to the latest version with the fix.