Closed Bug 702845 (rtl-meta) Opened 13 years ago Closed 3 years ago

[meta] RTL support for Firefox for Android

Categories

(Firefox for Android Graveyard :: General, enhancement, P5)

All
Android
enhancement

Tracking

(relnote-firefox 53+, fennec+)

RESOLVED INCOMPLETE
Tracking Status
relnote-firefox --- 53+
fennec + ---

People

(Reporter: nhirata, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: feature, meta)

Attachments

(1 file, 1 obsolete file)

- RTL will probably be needed in the future for L10n; would like to be able to accommodate for it now.

2 layouts (for tablets and phones) * no. of RTL languages + one default set for tablets and phones
Reading http://developer.android.com/guide/topics/resources/providing-resources.html#AliasResources, we need to reference the layouts each time, but implement it only once. menu is excluded, if I read that right, not sure if that's an issue even.

Also, for Hebrew, we'll need to use 'iw' as locale code for the layout, reminder.
Priority: -- → P4
Sriram - Can you look into what we need to do to support RTL?
Assignee: nobody → sriram
As of now, Android doesn't seem to support RTL languages. So, we might need to change the layout based on the language chosen by the user.

Say the user has set the language to be an RTL language, this can be stored in SharedPreferences and be read on startup. Based on whether the locale is RTL or not, we can supply the layout in onCreate() (eg, some_layout_ltr or some_layout_rtl).

However, there are few performance bottle necks. Opening a shared preference editor during onCreate adds an extra 250ms during startup, and that particular section of code has been blocked out.

The other option would be to specify layout as layout-en-US, layout-en-FR and so on. This makes the android decide the layout to use, and hence could be faster. However, since RTL languages aren't supported by android, I doubt if they will have the "qualifiers" supported in "layout-ab-CD" pattern.
Perhaps this could share some work with bug 702959.
Depends on: 702959
http://colincooper.net/?p=238 mentions support for various variants of Arabic, and Hebrew as supported languages on Android.

They render OK, too, looking at the tests I did with my test app on an Transformer tablet, https://wiki.mozilla.org/L10n:Mobile/Android/LocaleOSDeviceMatrix.

You have to do the trick with the 'iw' for hebrew as locale code during packaging, though, see also bug 700289.
(In reply to Axel Hecht [:Pike] from comment #5)
> http://colincooper.net/?p=238 mentions support for various variants of
> Arabic, and Hebrew as supported languages on Android.
> 
> They render OK, too, looking at the tests I did with my test app on an
> Transformer tablet,
> https://wiki.mozilla.org/L10n:Mobile/Android/LocaleOSDeviceMatrix.

I don't see any Arabic-script locales mentioned in the table there - does the device actually render Arabic properly in the UI? (See also bug 706888.)
http://developer.android.com/sdk/android-3.2.html#locs has arabic.

The wiki page only lists those locales that are obviously broken to my eye. For supported locales on android, I found the list on http://colincooper.net/?p=238 (linked from the wiki page) to be more useful.
Hello, could you please assign me to this bug. I am a member of the RTL team that will be completing this bug as part of our UCOSP student project with Mozilla. 

Thanks, 
Danielle
Assignee: sriram → icatchcows
Depends on: 924418
Depends on: 924699
Depends on: 925108
Depends on: 927667
Depends on: 928663
Depends on: 928688
Depends on: 935388
Depends on: 935392
Keywords: feature
https://dl.dropboxusercontent.com/u/26526390/fennec-28.0a1.en-US.android-arm.apk.zip

This is an apk of all of the patches related to this issuse applied! Feel free to play around with it.
Cleaning up some bug metadata. Unassigning for now until we un-stall this.
Assignee: icatchcows → nobody
Keywords: meta
Priority: P4 → --
Hardware: ARM → All
Summary: Allow for specifying RTL specific layouts → [meta] RTL support for Firefox for Android
Depends on: 1029644
Depends on: 1029646
Depends on: 1029649
Alright, well API Level 17 introduces "android:supportsRtl", with the introduction of "start" and "end" instead of "left" and "right", eg. "paddingStart" instead of "paddingLeft". It also enables "android:layoutDirection="locale"" in xml files. 

Obviously since only 46.6% of android devices support API level 17+, this alone is not at all sufficient. We can use "start" as well as "left" in order to still support all devices, but this still leaves the issue of RTL support on android devices that do not support API level 17.

I think the best solution is to have multiple xml layouts for both LTR and RTL layouts, which will make RTL support available on all devices. Shouldn't be too tedious with find & replace ;).
Thanks for the info Aymen. Could you point to a reference for the statistic you've cited? I'm particularly curious about which categories of devices support API level < 17.
https://developer.android.com/about/dashboards/index.html.

It's updated at least once a month. The data is collected from a 7-day period each month from devices that access the google play store. Obviously there are going to be devices that don't meet this criteria, but nonetheless it's going to be quite accurate.
Just to add, it's not very detailed. There is no link shown between the 3 different statistics shown there, which would be useful. Especially location data. There is also this website (http://androiddistribution.com/) which has the same source, but logs previous results in order to show growth/change.
(In reply to Aymen Qader from comment #11)
> Alright, well API Level 17 introduces "android:supportsRtl", with the
> introduction of "start" and "end" instead of "left" and "right", eg.
> "paddingStart" instead of "paddingLeft". It also enables
> "android:layoutDirection="locale"" in xml files. 
> 
> Obviously since only 46.6% of android devices support API level 17+, this
> alone is not at all sufficient. We can use "start" as well as "left" in
> order to still support all devices, but this still leaves the issue of RTL
> support on android devices that do not support API level 17.

Actually, to keep things as simple as possible, we want to only focus on Android API 17 and higher to begin. Once we get Firefox working on version of Android where RTL is supported, we can look at working backwards.

> I think the best solution is to have multiple xml layouts for both LTR and
> RTL layouts, which will make RTL support available on all devices. Shouldn't
> be too tedious with find & replace ;).

This will be harder than you think. Keeping the LTR and RTL layouts updated as new features are landed is harder than using the support built into API 17.
I'm going to make the firm assertion that we shouldn't try to address devices without rtl platform support. I have no desire for us to sign on to a painful game of inches — those earlier platform releases will have inadequate rendering support and missing scripts, and there's a strong incentive already for RTL users to use a more recent OS release.

Let's start by making Fennec work with, say, Fennec on an Arabic KitKat phone, and move on from there.
Alright, I guess we'll use the RTL features of API 17.

However, it's going to be quite messy, since we'll have to include both a "start" and "left" version of most formatting attributes.
(In reply to Aymen Qader from comment #17)
> Alright, I guess we'll use the RTL features of API 17.
> 
> However, it's going to be quite messy, since we'll have to include both a
> "start" and "left" version of most formatting attributes.

Agreed. Many of the "dependent" bugs already have patches and, yes, they use "start" and "left".
Depends on: 1070846
Depends on: 1081742
No longer depends on: 702959
Depends on: 1106800
latest stats on https://developer.android.com/about/dashboards/index.html shows that api 17+ is now up over 84% so it might be time to just support those devices and redo the patches to simplify.
and we are now narrowing the api versions supported in fennec 46 according to a post from rnewman in mobile-firefox

l;dr: Firefox for Android will no longer support Honeycomb (API 11-13) 

x86 builds will shortly shift to support only 15+, because the number of x86 devices running API 14 or lower is vanishingly small. That work is tracked in Bug 1062537.  

so rtl support differences would only vary on api 15 and 16.
Attached file RTL Firefox for Android review-v2.pdf (obsolete) —
Here is the RTL review/spec of Firefox for Android based on the discussion on dev.rtl[0]. The ODT version is also available on google docs[1].

[0] https://groups.google.com/forum/#!topic/mozilla.dev.rtl/HHl1Os6ScAM
[1] https://drive.google.com/open?id=0By6qFTjc3XvFVVNscElKSDdNTkE
Thanks Arash! Very helpful indeed. 
:rnewman: Could you please help get review/feedback/approval for these specs? Or let us know what the next good steps would be to get these implemented for Fennec? thanks!
Flags: needinfo?(rnewman)
Next steps are:

* sign-off from Anthony, Barbara, and Margaret
* filing specific bugs
Flags: needinfo?(rnewman)
Flags: needinfo?(margaret.leibovic)
Flags: needinfo?(bbermes)
Flags: needinfo?(alam)
Added it to the Fennec funnel for discussion about next steps and priorities
Flags: needinfo?(bbermes)
Flags: needinfo?(margaret.leibovic)
Flags: needinfo?(alam)
picking this up again, where are we at? 

Can we hand this over to the Taipei folks?
Flags: needinfo?(margaret.leibovic)
Flags: needinfo?(alam)
(In reply to Barbara Bermes [:barbara] from comment #25)
> picking this up again, where are we at? 
> 
> Can we hand this over to the Taipei folks?

There's a collection of bugs here that are in various states. You should talk to Delphine to see if anyone from the RTL community is willing to help report issues and test.
Flags: needinfo?(margaret.leibovic) → needinfo?(lebedel.delphine)
:Barbara, let's get in touch. Traveling now but will send you email.
Flags: needinfo?(lebedel.delphine)
Priority: -- → P5
(In reply to :Margaret Leibovic from comment #26)
> (In reply to Barbara Bermes [:barbara] from comment #25)
> > picking this up again, where are we at? 
> > 
> > Can we hand this over to the Taipei folks?
> 
> There's a collection of bugs here that are in various states. You should
> talk to Delphine to see if anyone from the RTL community is willing to help
> report issues and test.

I'm willing to volunteer.

FYI: In Hebrew version of Google Play Store, quite a noticeable amount of users are complaining about Firefox having no RTL/Hebrew support, while in fact the Hebrew locale is about 95% ready.
https://pontoon.mozilla.org/he/firefox-for-android-aurora
I'm guessing the Arabic and other RTL languages users are also wondering why there's no RTL support yet.
(In reply to ItielMaN from comment #28) 
> I'm willing to volunteer.
> 
> FYI: In Hebrew version of Google Play Store, quite a noticeable amount of
> users are complaining about Firefox having no RTL/Hebrew support, while in
> fact the Hebrew locale is about 95% ready.
> https://pontoon.mozilla.org/he/firefox-for-android-aurora
> I'm guessing the Arabic and other RTL languages users are also wondering why
> there's no RTL support yet.

That would be super helpful, thanks for jumping in here!
You are correct, many people from RTL languages are disappointed by lack of RTL support. I will reach out to you by email now and see if I can help you get connected to all this. Once more, thanks a lot for suggesting to help!
Seems like as per comment 23 from rnewman, the next steps are that Barbara and Anthony sign-off on the spec as well (should anyone else be pinged instead of Margaret?). I will then get in contact with RTL community to check these bugs are still valid (some are more than 3 years old), prioritize existing bugs, and also help file new ones. thanks!
Flags: needinfo?(bbermes)
Delphine, Sebastian would be the person in lieu of Margaret.

As per our offline email chats, I support this from a product perspective, please let Anthony know if there are any UX specific things that need to be discussed.

I'm also happy to review the updated bug list and confirm with you next steps
Flags: needinfo?(bbermes)
Depends on: 1298379
Depends on: 1298385
Depends on: 1298396
Depends on: 1298688
Depends on: 1298889
(In reply to Barbara Bermes [:barbara] from comment #31)
> Delphine, Sebastian would be the person in lieu of Margaret.
> 
> As per our offline email chats, I support this from a product perspective,
> please let Anthony know if there are any UX specific things that need to be
> discussed.
> 
> I'm also happy to review the updated bug list and confirm with you next steps

Thanks Barbara. :ItielMaN has been helping to prioritize and confirm existing bugs attached here. He's also started filing new bugs here, and is currently discussing rtl on android topic on the dev.rtl mailing list. I'll reach out to you and Vance with more details by email.

@Sebastian: As per Barbara's comment above, could you please help review the specs attached, and possibly approve them if they are looking good? thanks!
Flags: needinfo?(s.kaspari)
Depends on: 1298903
Depends on: 1298904
Depends on: 1298913
No longer depends on: 943751
No longer depends on: 933189
No longer depends on: 1081742
No longer depends on: 1106800
(In reply to Delphine Lebédel [:delphine - use need info] from comment #32)
> @Sebastian: As per Barbara's comment above, could you please help review the
> specs attached, and possibly approve them if they are looking good? thanks!

The spec and the new bugs are looking good to me. I don't have any experience with RTL languages myself, so I'll have to rely on you and the community to come up with a good list of bugs. But yeah, overall this looks sane and doable. I recommend to do the toolbar changes last; antlam is currently looking into re-designing parts of the toolbar.
Let's continue to chat in the child bugs as needed. :)
Flags: needinfo?(s.kaspari)
One thing to consider is that native RTL support was introduced in Android 4.2 [1] (Currently we support Android 4.0+). When implementing some of those features we might want to make them work on Android 4.2+ only if this is easier and we get a lot from the system for free. However when using some of those new APIs we need to make sure that everything's still fine for Android <= 4.2 devices (and LTR languages).

[1] http://android-developers.blogspot.de/2013/03/native-rtl-support-in-android-42.html
As this gets some momentum would be nice to have a tracking for it.
tracking-fennec: --- → ?
I'm in a similar situation with Sebastian. But I have given the spec a closer look and everything in the "priority 1" section makes sense to me. I think we can start with that list right now.

I do get a bit worried as we flip parts of the UI as stated in the P2 list but I think Sebastian makes a good point in comment 34. Since there's native RTL support in Android that we can take advantage of, we should defer to that where possible.
Flags: needinfo?(alam)
Made a small change and moved the "notifications with buttons" to list of bugs, due to recent discussions on RTL group.
Attachment #8734385 - Attachment is obsolete: true
I'm a native Hebrew speaker and I agree with Arash's updated specs.

Note that when RTLing the main menu, the back and forward buttons should do the opposite action (the right arrow button would go to the previous page, and the left arrow button would go forward).
I am gonna be the QA assigned for this and will track the testing process in this testplan: https://wiki.mozilla.org/QA/Fennec/Support_RTL
QA Contact: ioana.chiorean
Hi Sebastian, Barbara, is this planned for Fennec 50? I think this one shows up on Erin's slidedeck, specifically fennec roadmap for Fx50. I am wondering if there is a change of plan. 50 will hit Beta in a week and perhaps this needs to be retargeted for a later release.
Flags: needinfo?(s.kaspari)
Flags: needinfo?(bbermes)
(In reply to Ritu Kothari (:ritu) from comment #40)
> Hi Sebastian, Barbara, is this planned for Fennec 50? I think this one shows
> up on Erin's slidedeck, specifically fennec roadmap for Fx50. I am wondering
> if there is a change of plan. 50 will hit Beta in a week and perhaps this
> needs to be retargeted for a later release.

This is definitely wrong. This is for a future release TBD.
Flags: needinfo?(s.kaspari)
tracking-fennec: ? → +
Erin/Sebastian, this is an on-going task, so maybe instead of referencing the meta bug for 50, we should find a few sub-bugs that can be fixed by the Taipei team for 50. Do-able?

Vance, in case the team has already started to work on some of the RTL bugs, can you point us to the ones you feel could land in 50? 

Thanks
Flags: needinfo?(vchen)
Flags: needinfo?(s.kaspari)
Flags: needinfo?(elancaster)
Flags: needinfo?(bbermes)
> 
> Vance, in case the team has already started to work on some of the RTL bugs,
> can you point us to the ones you feel could land in 50? 
> 
> Thanks

Hi Barbara, we are still in the middle of the hiring process, so now as far as I know no one in Taipei is working on any RTL bug for Fennec
Flags: needinfo?(vchen)
Ok, makes sense, I moved it to 52 for now
Flags: needinfo?(s.kaspari)
Flags: needinfo?(elancaster)
Hi Sebastian,

Today I have a quick hack on Fennec to support RTL. And current findings are mostly very positive. As you mentioned in comment 34, with native framework support RTL, it cost almost nothing to fix all P1 issues. Major effort would be how to verify on old device and also LTR language(make sure that there is no side effect), and be careful on NewApi lint check result. Since many RTL issues can be resolved by one line of code. I think it would save a lot of time if I provide patch only on this issue. What do you think?
Flags: needinfo?(s.kaspari)
(In reply to Max Liu [:maliu] from comment #45)
> As you mentioned in comment 34, with native framework
> support RTL, it cost almost nothing to fix all P1 issues. Major effort would
> be how to verify on old device and also LTR language(make sure that there is
> no side effect)

If this is problematic on older devices and we end up in a situation where RTL support is only working satisfactorily on Android 4.2+ or 4.x devices then this is okay from my point of view too (We should talk to the product team in this case):

* It is still better than to not support RTL languages at all
* Android 5+ usage numbers are already ~58% and with Android 4.4 included it's > 80%
* Eventually usage numbers of those versions will go down and we can drop support for them

=> I think focusing on making use of the native support is the way to go for now.

> Since many RTL issues can be resolved by one line of code.
> I think it would save a lot of time if I provide patch only on this issue. What do you think?

What line of code is this? Is it the manifest flag?

This here is a meta bug and shouldn't be used for landing code. Instead we use it to group all related bugs (See "Depends on" section above). If we do not have a bug for this one line patch filed yet then file a new bug and link it to this meta bug. Then let's talk about the patch in the new bug.

After the new bug is fixed we should go through the other linked bugs and see if they are fixed now or if they need a specific patch. Most of the linked bugs are created by community contributors who speak a language that uses RTL writing systems - so we should consider them as minimum we need to support.
Flags: needinfo?(s.kaspari)
ni Barbara and Joe from product team per Comment#46 to weight-in the Pros/Cons of utilizing Android native RTL support.
Flags: needinfo?(jcheng)
Flags: needinfo?(bbermes)
Depends on: 1319302
Following bug 1319302, I can't see any difference whatsoever after updating Nightly to the latest build.
Is there a specific APK I need to install?
(In reply to ItielMaN from comment #48)
> Following bug 1319302, I can't see any difference whatsoever after updating
> Nightly to the latest build.

That landed 4 hours ago, it will be in tomorrow's nightly.
Indeed, today's Nightly is oriented for the first time right-to-left, but it's still in English.

Can we expect actual translations there any time soon? AFAIK at least some of the strings are already translated.
(In reply to Amir Aharoni from comment #50)
> Can we expect actual translations there any time soon? AFAIK at least some
> of the strings are already translated.

I assume you're testing Hebrew. I only see few strings missing for Fennec, so that doesn't sound right. I'd suggest to move this in a different bug though, given the number of people CCed, or to the l10n mailing list.
As Amir said, latest Nightly has some RTL changes, but still in English.
Unfortunately, the only visible RTL changes are within the New Tab page and the tab switcher UI. Also the toast notifications seems to be RTL'd now.
Depends on: 1321633
Depends on: 1321635
Testing with the Arabic versions from https://ftp.mozilla.org/pub/mobile/nightly/latest-mozilla-central-android-api-15-l10n/, all looks fine so far. The version from https://www.mozilla.org/en-US/firefox/channel/android/ does not seem to include any localization (not Arabic at least).
(In reply to Khaled Hosny from comment #53)
> The version from https://www.mozilla.org/en-US/firefox/channel/android/ does not seem to
> include any localization (not Arabic at least).

Arabic is not part of the multilocale build for Nightly, only very few locales are.
Depends on: 1321981
Depends on: 1322119
Depends on: 1322144
Depends on: 924703
Depends on: 924700
Depends on: 1323763
Depends on: 1323765
(In reply to Vance Chen [:vchen][skype:vance.lucida][vchen@mozilla.com] from comment #47)
> ni Barbara and Joe from product team per Comment#46 to weight-in the
> Pros/Cons of utilizing Android native RTL support.

Using Android native RTL support seems reasonable, assuming and as mentioned as well, we don't break anything for <4.2 users.

As per our Play Store data, and as Sebastian mentioned, ~85% of Fennec users are on >4.2.
Flags: needinfo?(bbermes)
Depends on: 1326291
Alias: rtl-meta
Depends on: 1331995
Depends on: 1332258
Depends on: 1336388
Depends on: 1325230
Flags: needinfo?(jcheng)
Depends on: 1337407
Depends on: 1337425
Depends on: 1337440
Depends on: 1337891
Depends on: 1337897
Depends on: 1337933
Depends on: 1337947
Depends on: 1338027
Depends on: 1338231
Depends on: 1331947
Depends on: 1339209
Depends on: 1324280
Depends on: 1322142
Depends on: 1347476
Depends on: 1347479
Depends on: 1348020
Depends on: 1350661
Depends on: 1351439
Depends on: 1354440
Depends on: 1358089
Depends on: 1361498
Depends on: 1407614
Depends on: 1415973
Depends on: 1432854
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Type: defect → enhancement
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.