68.44 KB, image/jpeg
46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
29.03 KB, image/png
32.32 KB, image/png
33.53 KB, image/png
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
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.
status-b2g-v2.2: --- → affected
Assignee: nobody → arthur.chen
Status: NEW → ASSIGNED
The order of the selectors corresponds to the order in the localized date time string. The correct order is year/day/month instead. The patch will be made based on that. : https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.ar.properties#L198
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.
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
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
Last Resolved: 4 years ago
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
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.
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.
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!
triage: P1, good clarifications in comment #13.
feature-b2g: --- → 2.2+
Priority: -- → P1
Created attachment 8561831 [details] 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) → 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
Last Resolved: 4 years ago → 4 years ago
status-b2g-master: affected → fixed
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+
status-b2g-v2.2: affected → fixed
Target Milestone: --- → 2.2 S6 (20feb)
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+]
status-b2g-master: fixed → verified
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
status-b2g-v2.2: fixed → verified
Test case has been added in moztrap: https://moztrap.mozilla.org/manage/case/15719/
You need to log in before you can comment on or make changes to this bug.