1432854 - Tab counter doesn't show native numbers for non-arabic numeral languages

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Suhail Alkowaileet]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171221220104

Steps to reproduce:

-Set Android to Arabic language.
-Open Firefox for Android.
-See tab count.


Actual results:

The shown numbers are (1, 2, 3...).


Expected results:

It should be (١, ٢, ٣...)

Comment 1

[Suhail Alkowaileet]

Attachment: fix_android_tab_count_to_native_numeral.patch

Comment 2

[Sorina Florean [:sflorean]]

I was able to reproduce this with Nexus 5 (Android 6.0.1) and Honor 8 (Android 7.0) on latest Nightly build and 59 Beta. Marking as NEW.

Comment 3

[Jan Henning [:JanH]]

Hello, thanks for the patch. I tested with a Try build from https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b1ed6ba1190ae9f3cddd9d620b818c7e365d32a&selectedJob=159421962 (use target.apk (2) for the Arabic single-locale build) and it looks fine.

Two issues though:
1. Can you please upload a complete patch including a commit message (Bug 1432854 - <Brief description of your patch>) and comitter details (name and email) please?
2. For bonus points - with your current patch, the tab count display isn't updated after changing the locale and continues using the old number style until the next tab event occurs. Could you fix that as well? You should be able to start at [1] to notify the BrowserToolbar about the locale change and then forward this until you reach the tab counter.

[1] https://dxr.mozilla.org/mozilla-central/rev/5454ed95c82a956009db2f4b04d008ec8753e61e/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#2823

Comment 4

[Suhail Alkowaileet]

Attachment: Bug 1432854 - Fix Tab counter doesn't show native numbers for non-arabic numeral languages

Review commit: https://reviewboard.mozilla.org/r/218288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218288/

Comment 5

[Suhail Alkowaileet]

(In reply to Jan Henning [:JanH] from comment #3)
Hello Jan,
Thanks for this great reply. It really helped me. Now I did fix to notify after changing the locale but I'm not sure if removing the condition check in TabCounter[1] will be OK 

Comment 6

[Jan Henning [:JanH]]

(In reply to Suhail Alkowaileet from comment #5)
> It really helped me. Now I did fix to notify
> after changing the locale but I'm not sure if removing the condition check
> in TabCounter[1] will be OK

Well yeah, that tab event sort of does the trick, but doesn't fit exactly what you need. In the general case having that check is still necessary I think, so you should rather leave it in place.
Instead, just add an onLocaleReady method in the BrowserToolbar and from there call the TabCounter's plain setCount method.

Comment 7

[Suhail Alkowaileet]

Attachment: Bug 1432854 - Fix Tab counter doesn't show native numbers for non-arabic numeral languages

Review commit: https://reviewboard.mozilla.org/r/218734/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218734/

Comment 8

[Suhail Alkowaileet]

(In reply to Jan Henning [:JanH] from comment #6)
Thank you Jan!

Comment 9

[Jan Henning [:JanH]]

Comment on attachment 8949396 [details]
Bug 1432854 - Fix Tab counter doesn't show native numbers for non-arabic numeral languages

https://reviewboard.mozilla.org/r/218734/#review225024

Looks fine, thanks.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64cdff8ffa9b
Fix Tab counter doesn't show native numbers for non-arabic numeral languages r=JanH

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/64cdff8ffa9b

Comment 12

[Sorina Florean [:sflorean]]

Verified as fixed on latest Nightly build.
Device: Samsung Galaxy S8(Android 7.0) and Nexus 6P(Android 8.1.0).

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

It's late in the 59 beta cycle now, so I think it's best for this change to stay in the 60 release.