+++ This bug was initially created as a clone of Bug #702845 +++ - 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

Comment on attachment 8813002 [details] Bug 1319302 - RTL support for Firefox for Android, https://reviewboard.mozilla.org/r/94520/#review95226 LGTM. Maybe it would be helpful to send a mail to the mobile-firefox-dev mailing list to let other developers know what they should be mindful of to support RTL in the future and do not break anything you just "fixed" (Best practices?). After this lands in Nightly let's ask ItielMaN in bug 702845 to verify which linked bugs are now fixed (on Android >= 4.4) and what is still needed. ::: mobile/android/base/java/org/mozilla/gecko/tabs/TabPanelBackButton.java:18 (Diff revision 5) > +import android.support.v4.graphics.drawable.DrawableCompat; > +import android.support.v4.view.MarginLayoutParamsCompat; > +import android.support.v4.view.ViewCompat; > +import android.support.v4.view.ViewGroupCompat; > import android.util.AttributeSet; > +import android.util.Log; nit: Unused import ::: mobile/android/base/java/org/mozilla/gecko/toolbar/PhoneTabsButton.java:36 (Diff revision 5) > + int layoutDirection = ViewCompat.getLayoutDirection(this); > + > + Point[] nodes = getDirectionalNodes(width, height, layoutDirection); > + TabCurve.Direction directionalCurve = getDirectionalCurve(layoutDirection); nit: final ::: mobile/android/base/java/org/mozilla/gecko/toolbar/PhoneTabsButton.java:63 (Diff revision 5) > + nodes = new Point[]{ > + new Point(width, 0) > + , new Point(width, height) > + , new Point(0, height) > + , new Point(0, 0) > + }; > + } else { > + nodes = new Point[]{ nit: style, space before { ::: mobile/android/base/java/org/mozilla/gecko/widget/FadedSingleColorTextView.java:42 (Diff revision 5) > mTextGradient.getColor() != color || > mTextGradient.getWidth() != width); > > final boolean needsEllipsis = needsEllipsis(); > if (needsEllipsis && needsNewGradient) { > - mTextGradient = new FadedTextGradient(width, fadeWidth, color); > + boolean isRTL = ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL; nit: final ::: mobile/android/base/java/org/mozilla/gecko/widget/ResizablePathDrawable.java:15 (Diff revision 5) > import android.graphics.Color; > import android.graphics.Paint; > import android.graphics.Path; > import android.graphics.drawable.ShapeDrawable; > import android.graphics.drawable.shapes.Shape; > +import android.support.v4.graphics.drawable.DrawableCompat; Unused import ::: mobile/android/base/java/org/mozilla/gecko/widget/themed/ThemedImageButton.java:20 (Diff revision 5) > import android.content.Context; > import android.content.res.ColorStateList; > import android.content.res.TypedArray; > import android.graphics.drawable.ColorDrawable; > import android.graphics.drawable.Drawable; > +import android.support.v4.graphics.drawable.DrawableCompat; Unused import

(In reply to Sebastian Kaspari (:sebastian) from comment #6) > After this lands in Nightly let's ask ItielMaN in bug 702845 to verify which > linked bugs are now fixed (on Android >= 4.4) and what is still needed. No problem. Just let me know when the changes will land in Nightly. I can also test in Android 4.1.x, my family has some old devices. Tell me if that will be needed.

(In reply to ItielMaN from comment #7) > (In reply to Sebastian Kaspari (:sebastian) from comment #6) > > After this lands in Nightly let's ask ItielMaN in bug 702845 to verify which > > linked bugs are now fixed (on Android >= 4.4) and what is still needed. > > No problem. Just let me know when the changes will land in Nightly. > I can also test in Android 4.1.x, my family has some old devices. Tell me if > that will be needed. We are relying on some Android platform features that have been introduced in Android 4.2 - 4.4. So with the changes here we should be in a much better shape on Android >= 4.4. With usage numbers of earlier Android versions going down it is unlikely that we will invest a lot of time to support older versions.

Pushed by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/120c84ca5362 RTL support for Firefox for Android, r=sebastian

(In reply to Max Liu [:maliu] from comment #0) > +++ This bug was initially created as a clone of Bug #702845 +++ Your bug summary and patch commit summary does not contain enough information, to a point that GMail simply confused and combine the two threads. Please make sure to change the bug summary and make your intention clear next time!

ni L10N team Delphine such that L10N team can be on the same page of the current Fennec RTL status.

Thanks everyone for the effort. I just downloaded the latest Nightly[1] for Persian locale to test the changes and update the RTL spec, but I cannot see any difference in the UI. Do I need to toggle any option or something? [1] http://ftp.mozilla.org/pub/mobile/nightly/latest-mozilla-central-android-api-15-l10n/fennec-53.0a1.fa.android-arm.apk

Hi Max, could you check and feedback on Comment#16 ?

Hi Arash, For now, we support RTL by android framework native support. To see the difference, a. Android device system language must be RTL; b. App have to build with RTL locale. In your case, the apk is RTL ready. Would you help to confirm if your device language is also set to RTL(ex. Persian) please?

Thanks Max, one more question: Once we leverage the Android native RTL support, which Priority 1 parts of the following spec have been done ? https://bug702845.bmoattachments.org/attachment.cgi?id=8787324 Thanks

Thanks Max. I can confirm that with changing the whole phone's/tablet's language to Persian, most issues specified on RTL specs are fixed, except these issues happening only on tablet screens: 1) positions of tabs are still from left to right (first issue on page 3) 2) back/forward icon directions (first issue on page 3) 3) The lock icon/button (for SSL pages) overlaps forward button and makes it unusable (new issue not in specs) The interface for phones seems pretty perfect. Isn't it possible to force the direction when choosing an RTL language?

My question in Comment#19 is answered by Arash in Comment#20, but I will keep the ni for Max since still need his comment on Comment#20

(In reply to Arash Mousavi [:Arash-M] from comment #20) > Thanks Max. I can confirm that with changing the whole phone's/tablet's > language to Persian, most issues specified on RTL specs are fixed, except > these issues happening only on tablet screens: > > 1) positions of tabs are still from left to right (first issue on page 3) Yes, this part is implemented by custom view, no native RTL support. So is WIP > 2) back/forward icon directions (first issue on page 3) > 3) The lock icon/button (for SSL pages) overlaps forward button and makes it > unusable (new issue not in specs) I noticed these two symptom during development, but I'm pretty sure they should both be fixed before patch landed. I also cannot reproduce the symptom on current nightly build. Would you help to provide more detailed information on Bug 928688 please? > > The interface for phones seems pretty perfect. Isn't it possible to force > the direction when choosing an RTL language? This part is also under investigation. I'll file another bug to keep track on this.

(clearing ni off me since this is on our radar)

Release Note Request (optional, but appreciated) [Why is this notable]: Support RTL on Fennec firefox53 [Affects Firefox for Android]: [Suggested wording]:RTL Support on Fennec firefox53 [Links (documentation, blog post, etc)]: https://wiki.mozilla.org/Mobile/Firefox_for_Android/RTL

Thanks, I updated the release note for 53 beta with a link to the wiki page.