981598 - [System]: add RTL support to Time Picker

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Francesco Lodolo [:flod]]

Localizability of time picker in System has been improved in bug 932356, but it's still lacking proper RTL support.

Comment 1

[Francesco Lodolo [:flod]]

Ahmed, is this something you can take a look into or have suggestions on what the expected behavior should be?

Some notes:
* bug 932356: the reference used to build the time picker is shortTimeFormat (/shared/date/date.properties), for ar this key is currently set to "%I:%M %p".
* bug 932270: this let locales translate AM/PM, but those strings are still missing from the Arabic localization.

I tried running a profile in Firefox and the time reel is just displayed like the English one.

Comment 2

[Ahmed Nefzaoui [:Nefzaoui]]

To answer the question of what do I think the behavior should be; Normally in RTL, AM/PM should be on the left.
And according to this attachement:
https://bug932356.bugzilla.mozilla.org/attachment.cgi?id=8384270
3rd screenshot on the left makes sense.
I will try and have a look at it tonight in a device and get back with feedback or a PR :)

Comment 3

[Francesco Lodolo [:flod]]

Attachment: ar.png

Ahmed, is this correct?

Comment 4

[Ahmed Nefzaoui [:Nefzaoui]]

Yes, it is! :)

Comment 5

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8460957 [details]
ar.png

Only shifting the AM/PM label to the left.
Yes it is correct. :)

Comment 6

[Francesco Lodolo [:flod]]

Attachment: Pull request on Github

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8460986 [details] [review]
Pull request on Github

Thanks for the patch!

Comment 8

[Carsten Book [:Tomcat]]

flod, tim, are the travis errors on https://travis-ci.org/mozilla-b2g/gaia/builds/30671392 are related to this patch (i guess no, but better ask :)

Comment 9

[Francesco Lodolo [:flod]]

This patch changes the aspect of time picker only for RTL locales, so it's probably not part of any tests at the moment (RTL support is not great at the moment).

Comment 10

[Carsten Book [:Tomcat]]

landed in master as https://github.com/mozilla-b2g/gaia/commit/ce38a85582b3fa09021e8c5b37394b543dd39890

Comment 11

[Francesco Lodolo [:flod]]

Verified on flame

Gaia      c8511217183ade630f5afa7698c04efef5c6806b
Gecko     https://hg.mozilla.org/mozilla-central/rev/a4dcfbebcb58
BuildID   20140728040209
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014

Comment 12

[Peter Bylenga [:PBylenga]]

Mass Edit: adding the [rtl-meta]