quran / quran_android

a quran reading application for android
http://android.quran.com
GNU General Public License v3.0
2.01k stars 888 forks source link

Audio panel does not show the sura names properly (in Farsi) #849

Closed bateni closed 7 years ago

bateni commented 7 years ago

This is assuming I have patched the updated Farsi translations: #848 #847 But it should occur without those, as well.

Unlike English and Arabic interfaces, sura names are not fully shown. See the picture (and ignore the green-ish circles).

screen shot 2017-04-15 at 9 22 18 pm

Then I thought maybe the problem is that there are no customized Farsi layouts. I don't know what is the difference between layouts under layout-ar and layout, but in principle Farsi and Arabic layouts should be exactly the same (except for the localized strings).

So I copied Arabic layouts under layout-fa directory, etc. This does not have the above patch for Farsi translations, but the result is similar. See the picture.

screen shot 2017-04-15 at 9 10 19 pm

The spinner's width is good, and can theoretically fit the longer sura names. But even a short name like "الرعد" is not fully displayed.

nyitgroup commented 7 years ago

I can't replicate this problem on my phone on the release or development version. Which version of the App are you using? Can you try the latest Dev version?

It might be related to this spinner issue? https://github.com/quran/quran_android/commit/1de170c3bb9884ab5d6643880c662b6c71734c6e

bateni commented 7 years ago

I can reproduce this with the latest version from github. I just open the project in Android Studio and run the Debug binary on a simulator for Nexus 5, API 25 (with locale set to fa-IR).

screen shot 2017-04-17 at 6 13 14 pm
nyitgroup commented 7 years ago

confirmed fati7ah_device-2017-04-18-200834

nyitgroup commented 7 years ago

fixed device-2017-04-18-210846

I will upload potential fix soon

ahmedre commented 7 years ago

unfortunately, this is a pretty tricky problem. here are some notes:

  1. if you look at the layout, you'll see that the text doesn't quite fill its parent (the Spinner), even though it should.
  2. in QuranSpinner, we override onMeasure to correctly figure out the width of the dropdown - if you notice, the dropdown width is correct because of this. both Spinner and AppCompatSpinner calculate the dropdown width (and the spinner's width) based on the largest width of the first 15 elements, whereas QuranSpinner calculates the last 15 items, and compares that width to the width found from the first 15 items, and uses the largest of the 2.
  3. if we disable all the logic in QuranSpinner, the bug still happens, suggesting it's a bug elsewhere.
  4. if we ask the child to measure in QuranSpinner's onMeasure once we decide on a new width to use, this gets overridden later on by Spinner's setUpChild method, which measures the view using the Spinner's own layout parameters as a basis.

the least hacky workaround i could come up with here is to explicitly update the spinner's layout parameters once we've determined the width. this will cause any further setUpChild calls to use the spinner's current layout parameters, which have a fixed width, and consequently, it solves the problem. i'll push a patch for this insha'Allah.

as for your question - "why does it work for English and Arabic, but not for Farsi?" i think the reason is that there is some bug in the text measurement logic, specifically when you combine Farsi numbers with Farsi text. here are 2 proofs for this:

  1. remove the for loop at AyahPlaybackFragment, line 170, which prepends the localized number to the sura name - re-run the app and you'll see that it now works, just like Arabic and English does.
  2. undo change 1 if you made it, and then jump into the QuranUtils.getLocalizedNumber method, and force isArabicNames to true, then re-run. you'll again see that it works just fine.

update was playing around with this some more, and noticed a few other interesting things - removing the . between the number and text fixes the problem also, as does replacing it with a -, for example. leaving the . but explicitly putting some English (ex zzz) also works.