Closed Bug 1129630 Opened 9 years ago Closed 9 years ago

[RTl][Settings] The date is in the wrong order on the picker wheel in settings

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(feature-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S6 (20feb)
feature-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: KTucker, Assigned: arthurcc)

References

Details

Attachments

(5 files, 1 obsolete file)

Description:
When using a RTL language, the user will notice the date is in the wrong order on the picker wheel in settings. 

Repro Steps:
1) Update a Flame to 2015020401022
2) Change the language to Arabic.
3) Open settings and tap on "Date & Time".
4) Tap on "Date".
5) Observe the order of the date on the wheel

Actual:
The date appears in the wrong order.

Expected:
The date appears in the correct order. Year/Month/Day

Environmental Variables:
Device: Flame 3.0 (Full Flash)(KK)(319mb)
Build ID: 20150204010225
Gaia: dfebaaa8aab43470f482d09d71137bab840c3ae9
Gecko: 0c2f7434c325
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 3/3, 100%
See attached: screenshot
Blocks: settings-rtl
No longer blocks: camera-rtl
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
No longer depends on: 1129586
This issue also occurs on the Flame 2.2

The date order is not correct on the picker wheel.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150204002509
Gaia: a4c4cc86303a554facb8f45b7e764e5c4473c3de
Gecko: 8669c26fd4a5
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.
Flags: needinfo?(pbylenga)
Attached image IMG_0499.JPG
Assignee: nobody → arthur.chen
Status: NEW → ASSIGNED
The order of the selectors corresponds to the order in the localized date time string[1]. The correct order is year/day/month instead. The patch will be made based on that.

[1]: https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.ar.properties#L198
Attached image screenshot after patch (obsolete) —
Comment on attachment 8560386 [details] [review]
[PullReq] crh0716:1129630 to mozilla-b2g:master

Alive, not sure who is the owner of the style. Could you help review it or redirect to someone who is able to do the review? Thanks.
Attachment #8560386 - Flags: review?(alive)
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(pbylenga)
This is not a bug, as filed. The spec notes year/month/day for the date display, and this has been correctly shown for quite some time now. If the date format is not year/month/day, it is a regression.

Resolving invalid, ni? Josh for another example of deviation from patterns and bugs that should not be filed.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: needinfo?(jocheng)
Resolution: --- → INVALID
Patch is for year/day/month according to screenshot and is not correct.
Reopening so we can discuss in the next triage. 

:kaze, I'd like your input on this one. The patch here was made to agree with the ar.properties date format (see comment #3), but that format disagrees with the spec which says the order should be Y/M/D.
Status: RESOLVED → REOPENED
Flags: needinfo?(fabien)
Resolution: INVALID → ---
Stephany,
The attached picture https://bugzilla.mozilla.org/attachment.cgi?id=8559402 is M/D/Y which I think is still a bug against RTL spec as Y/M/D.
Flags: needinfo?(jocheng)
Comment on attachment 8560386 [details] [review]
[PullReq] crh0716:1129630 to mozilla-b2g:master

Looks like there is no final decision made yet, cancel the review at first.
Attachment #8560386 - Flags: review?(alive)
Note that the date selector for all other LTR languages follows the order of the date string in the language file. I think we should do the same thing for RTL languages.
AFAIK, none of these two screenshots are OK. It should be year/month/day — and to be sharp, it should be %d/%m/%Y with the RTL reading order.

Quite disturbing, the shared Arabic l10n file for date/time formats has been wrong from the very beginning (i.e. the en-US default has been kept):

https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.ar.properties#L194-L203

(In reply to Arthur Chen [:arthurcc] from comment #12)
> Note that the date selector for all other LTR languages follows the order of
> the date string in the language file. I think we should do the same thing
> for RTL languages.

Exactly. Just remember to mirror the selector order in RTL mode.

It would be nice if that patch included a fix for shared/locales/date/date.ar.properties as well, so it can be tested properly by developers. I believe the correct strings would be:

  # Date/Time format
  # see http://pubs.opengroup.org/onlinepubs/007908799/xsh/strftime.html
  dateTimeFormat_%c = %a %e %b %Y %I:%M:%S %p
  dateTimeFormat_%x = %d/%m/%Y

Ahmed, can you please confirm and make sure this matches the Arabic locale in the l10n repository?
Flags: needinfo?(fabien) → needinfo?(nefzaoui)
Yes, that's correct Kazé.
Changes has been pushed to the locamotion server and should show up in gaia-l10n/ar of hg in an hour or so.
Thanks!
Flags: needinfo?(nefzaoui)
triage: P1, good clarifications in comment #13.
feature-b2g: --- → 2.2+
Priority: -- → P1
Attached image updated screenshot
Screenshot of the updated PR.
Attachment #8560382 - Attachment is obsolete: true
Comment on attachment 8560386 [details] [review]
[PullReq] crh0716:1129630 to mozilla-b2g:master

The PR was updated based on comment 13. Alive, Kaze, could you help take a look at the patch? Thanks.
Attachment #8560386 - Flags: review?(alive)
Attachment #8560386 - Flags: feedback?(fabien)
Attachment #8560386 - Flags: review?(alive) → review+
Comment on attachment 8560386 [details] [review]
[PullReq] crh0716:1129630 to mozilla-b2g:master

Thanks for the quick update, the result is fine to me.

(I left a couple nitpicks on your PR, nothing critical)
Attachment #8560386 - Flags: feedback?(fabien) → feedback+
Thanks Alive and Kaze.

Kaze, I did not change all the autos to unset for aligning to the styles for LTR mode. I set the values to unset only when the corresponding property does not have any value.

master: 14d3ea7df3d8e8bd14f326fa15b8f017044d79f9
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Comment on attachment 8560386 [details] [review]
[PullReq] crh0716:1129630 to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: The order of the date selectors is incorrect.
[Testing completed]: Testing on the device
[Risk to taking this patch] (and alternatives if risky): Low as it only adds styles for RTL mode.
[String changes made]: None
Attachment #8560386 - Flags: approval-gaia-v2.2?
Attachment #8560386 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
This issue hase been verified successfully on Flame 3.0.
Reproduce rate:0/5
Attchment:Verify_RTL_Date.png.

Flame 3.0:

Gaia-Rev        d5a71cedb37dd45f439f672489db3994b349ac43
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/3094601af679
Build-ID        20150212010213
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150212.042740
FW-Date         Thu Feb 12 04:27:51 EST 2015
Bootloader      L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage+][rtl-impact], [MGSEI-Triage+]
Attached image Verify_RTL_Date.png
This issue has been verified successfully on Flame 2.2.
Attachment:Verify_2.2.png

Flame 2.2 build:

Build ID               20150215002504
Gaia Revision          ea64caf6d4ab03fc4472eca9f41f20d651d55fa9
Gaia Date              2015-02-13 05:27:43
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/62c80c92b39e
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150215.040852
Firmware Date          Sun Feb 15 04:09:03 EST 2015
Bootloader             L1TC000118D0
Status: RESOLVED → VERIFIED
Attached image Verify_2.2.png
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15719/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: