935388 - RTL support for history page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Lauren]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36

Steps to reproduce:

This bug blocks on 702845; all the padding on the history page tab widget should be in the reverse direction when in RTL locale

Comment 1

[Lauren]

Attachment: historyPage_rtl.patch

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 827854 [details] [diff] [review]
historyPage_rtl.patch

Throwing review ot Lucas while Sriram is PTO.

Question: Does paddingStart work on older versions of Android or do we need both paddingStart and paddingLeft ?

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 827854 [details] [diff] [review]
historyPage_rtl.patch

Review of attachment 827854 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch! Giving r- to get some answers. Looks fine *if* it doesn't break anything on pre-17 Android.

::: mobile/android/base/resources/values-large-land-v11/styles.xml
@@ +46,5 @@
>  
>      <style name="Widget.Home.HistoryTabWidget">
>          <item name="android:showDividers">beginning|middle|end</item>
>          <item name="android:dividerPadding">0dp</item>
> +        <item name="android:paddingStart">100dp</item>

Isn't this going to break on pre-17 Android? Or is this made backwards-compatible at build time?

Comment 4

[Lauren]

Attachment: bug-935388-fix

Comment 5

[Lauren]

Woops, that's right - I've uploaded a new patch keeping the paddingLeft attribute.

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 8341787 [details] [diff] [review]
bug-935388-fix

Review of attachment 8341787 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, assuming the paddingStart attribute is safely ignored on pre-17 Android devices.

Comment 7

[Lauren]

Yep! It actually is. They actually recommend that you keep both paddingStart and paddingLeft attributes to support pre and post 17 devices: http://android-developers.blogspot.ca/2013/03/native-rtl-support-in-android-42.html

Comment 8

[Itiel]

Looking pretty good on the latest Nightly build, however there are 2 minor issues.
The first is where long hisotry item's titles/URLs are being faded out from the wrong side (or not faded at all if the text is hebrew).
Scenarios to consider:
1. Long URLs should ALWAYS fade away from the right side, no matter if the build is RTL or LTR
2. Long history item's title should be faded away according to it's text directionality:
   a. If text is RTL, fade out from the left (currently there's no fading at all)
   b. If text is LTR, fade out from the right
   * The same should possibly be applied to LTR builds as well

The second is that the strings indicating the time the history items were created are still LTR ("Today", "Last 7 days" etc).

I may file different bugs on these two.

Comment 9

[Rachelle Yang [:ryang][[e-mail]]]

Hi ItielMaN , 

Thank you for your support! could you please file another 2 bugs mentioned in comment8, and share with us the bugID!

Thank you very much !

Comment 10

[Reza Habibi (rehb)]

Attachment: History-RTL-Fixed.png

Comment 11

[Itiel]

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #9)
> Hi ItielMaN , 
> 
> Thank you for your support! could you please file another 2 bugs mentioned
> in comment8, and share with us the bugID!
> 
> Thank you very much !

Done.

Comment 12

[Sorina Florean [:sflorean]]

Hi all,

Tested on latest Nightly (Arabic) and the results are here:
Phone:
- LG G4 (Android 5.1) - https://i.imgur.com/yKu00bZ.png
Tablet:
- Asus ZenPad 8 (Android 6.0.1) - https://i.imgur.com/Pg0KzGQ.png

We will test again after bug 1321635 and bug 1321633 are fixed.

Comment 13

[Ioana Chiorean]

This is verified fixed with the fixed from bug 1321635 and bug 1321633 too. 
Nightly AR 53.0a1 2017-01-17

Comment 14

[Ioana Chiorean]

This was implemented and works correctly - further bugs that might arise will be linked for tracking.