[B2G][L10n][l12y][Gallery][Video] Taken/Shoot Date in image/video info not localizable

VERIFIED FIXED in Firefox OS v1.3

Status

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: petercpg, Assigned: flod)

Tracking

({l12y})

unspecified
1.4 S1 (14feb)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: LocRun1.3)

Attachments

(4 attachments)

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
Created attachment 8371645 [details]
Screenshot for thumbnail mode

Oops, I forgot to attach screenshot.
Created attachment 8371646 [details]
Screenshot for picture info
(Assignee)

Comment 4

5 years ago
Ok, much clearer, and I think also much harder to fix :-\
(Assignee)

Comment 5

5 years ago
(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.
Blocks: 938417
(Assignee)

Comment 6

5 years ago
Created attachment 8372110 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16071

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)
(Assignee)

Comment 7

5 years ago
Created attachment 8372113 [details]
Test before/after on Keon

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)

Updated

5 years ago
blocking-b2g: --- → 1.3?
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.
Keywords: qawanted
Jason: we didn't display dates before 1.3, so there was not a date localization issue before.

Updated

5 years ago
Keywords: qawanted
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+
Flags: needinfo?(pdahiya)
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.
Flags: needinfo?(pdahiya)
(Assignee)

Comment 15

5 years ago
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
(Assignee)

Comment 16

5 years ago
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.
Flags: needinfo?(pdahiya)
(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.
Flags: needinfo?(pdahiya)
(Assignee)

Comment 18

5 years ago
Thanks, Travis passed this time.

Setting keyword since I don't have access to Gaia.
Keywords: checkin-needed
Master: ee5976ddf603d64ff0f3b67ff246a741cba0845e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-b2g-v1.3: --- → affected
status-b2g-v1.4: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
1.3 blockers no longer have auto-approval to land. Please request gaia-v1.3 approval on the patch for uplift.
(Assignee)

Comment 21

5 years ago
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)

Updated

5 years ago
Attachment #8372110 - Flags: approval-gaia-v1.3?(jsmith) → approval-gaia-v1.3?(fabrice)

Updated

5 years ago
Attachment #8372110 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
v1.3: aee7ae4c598a781f1d811e9082261a16ada58532
status-b2g-v1.3: affected → fixed
(Assignee)

Comment 23

5 years ago
Verified on Keon master (1.4 2014-03-01 git commit f6f23511).
Status: RESOLVED → VERIFIED
status-b2g-v1.3T: --- → fixed
You need to log in before you can comment on or make changes to this bug.