Closed Bug 968071 Opened 7 years ago Closed 7 years ago
[B2G][L10n][l12y][Gallery][Video] Taken/Shoot Date in image/video info not localizable
105.81 KB, image/png
74.11 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review|
125.06 KB, image/png
Expected Results: "2013-11-09", yyyy-mm-dd Actual Results: "11-09-2013", mm-dd-yyyy Device: Geeksphone Keon Build: 20140205010019 Gaia: 3405205 Platform Version: 28.0
I was about to file the same bug. I think this is indeed a real issue because the date may be misread. On this panel, some fields are localized (the size unit, for instance), while this one is not, and the user has no way to know in which format the date is. We have a poor user experience there, sadly.
Summary: [B2G][L10n][Gallery] Chinese Traditional: Taken/Shoot Date in image/video info not localizable → [B2G][L10n][l12y][Gallery][Video] Taken/Shoot Date in image/video info not localizable
Oops, I forgot to attach screenshot.
Ok, much clearer, and I think also much harder to fix :-\
(In reply to Francesco Lodolo [:flod] from comment #4) > Ok, much clearer, and I think also much harder to fix :-\ Sorry, wrong bug (this comment was for bug 932356). Too many bugs open. I'm looking into this to see if I can understand the code.
This approach let us reuse an existing string, so it can be ported to 1.3 Note that this also changes the en-US format from mm-dd-YYYY to mm/dd/YYYY
Attachment #8372110 - Flags: review?(dale)
Tested locally on Keon, works as expected
Comment on attachment 8372110 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16071 Same, moving review to gallery peer
Attachment #8372110 - Flags: review?(dale) → review?(dflanagan)
Actually, let's check something before the nom. Did we ever support a localized format here?
blocking-b2g: 1.3? → ---
(In reply to Jason Smith [:jsmith] from comment #5) > Actually, let's check something before the nom. Did we ever support a > localized format here? i.e. QA Wanted to see what happens with 1.2 with locales that produce an alternative date format.
Jason: we didn't display dates before 1.3, so there was not a date localization issue before.
Comment on attachment 8372110 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16071 Thank you for fixing this. And my sincere apologies that it needed fixing in the first place. I think I reviewed the bad code in media_utils.js and should have caught that. I did not understand how shared/js/l10n_date.js worked, and did not know that we could use the string dateTimeFormat_%x Jason: I certainly support uplifting this patch to 1.3. Punam: please take a look at this patch, since, I think you may have been the original author of the media_utils.js file.
Attachment #8372110 - Flags: review?(dflanagan) → review+
bbajaj & I had a discussion offline on a blocking decision here - we agree to block on this because this is bad user impact for localization, as users will always get exposed to dates not matching their country-specific date format within the gallery app.
blocking-b2g: --- → 1.3+
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16071 Patch looks good. There is a lint error in apps/video/test/unit/mock_l10n.js because of which travis build is failing https://travis-ci.org/mozilla-b2g/gaia/jobs/18432799 With that fixed, it's good to merge.
Thanks everyone. I had no idea what "lint" was (I thought it was some random failure...). I fixed the errors, squashed commits and pushed. Waiting for Travis to complete, if it passes I'm going to set the keyword checkin-needed
Assignee: nobody → francesco.lodolo
Status: NEW → ASSIGNED
Travis failed but it seems completely unrelated https://travis-ci.org/mozilla-b2g/gaia/builds/18711327 Not sure if setting checkin-needed is a good idea with these conditions.
(In reply to Francesco Lodolo [:flod] from comment #16) > Travis failed but it seems completely unrelated > https://travis-ci.org/mozilla-b2g/gaia/builds/18711327 > > Not sure if setting checkin-needed is a good idea with these conditions. The travis failure on this build is unrelated, i have started the build again on https://travis-ci.org/mozilla-b2g/gaia/jobs/18711331 and hopefully it should pass this time.
Thanks, Travis passed this time. Setting keyword since I don't have access to Gaia.
1.3 blockers no longer have auto-approval to land. Please request gaia-v1.3 approval on the patch for uplift.
Comment on attachment 8372110 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16071 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): hard-coded date format in code [User impact] if declined: date in media info is not localized/localizable and unclear to non English users [Testing completed]: - [Risk to taking this patch] (and alternatives if risky): very low [String changes made]: none (reusing existing shared string)
Attachment #8372110 - Flags: approval-gaia-v1.3?(jsmith)
Attachment #8372110 - Flags: approval-gaia-v1.3?(jsmith) → approval-gaia-v1.3?(fabrice)
Attachment #8372110 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
Verified on Keon master (1.4 2014-03-01 git commit f6f23511).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.