Closed Bug 1197454 Opened 9 years ago Closed 9 years ago

Update Video to be ready for L10n API v3

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

Mostly remove mozL10n.get and l10n_date to Intl
Attached file pull request (deleted) —
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment on attachment 8651323 [details] pull request this patch completes the changes in Media apps (Gallery has been refactored in bug 1170973 and Camera in bug 1197019). David, can you review it?
Attachment #8651323 - Flags: review?(dflanagan)
:djf, of course I only wanted to ask you to review the latter patch. The first in this PR is being reviewed by :stas in bug 1208698.
Depends on: 1208698
Comment on attachment 8651323 [details] pull request Transferring the review to Punam.
Attachment #8651323 - Flags: review?(dflanagan) → review?(pdahiya)
Comment on attachment 8651323 [details] pull request Hi Zibi Patch looks good and has my r+ . Its verified for gallery and video changes. A nit noted in github on comment. I observed in video app similar as noted in gallery group headers missing support for 'run time mirrored' pseudo locale (https://bugzilla.mozilla.org/show_bug.cgi?id=1170973#c7) Also, did you happen to check video app launch performance with the patch. For some reason I can't see raptor results on tree herder. Thanks for the patch!
Attachment #8651323 - Flags: review?(pdahiya) → review+
Sure, I tested it on Flame-kk, runs=31 Master: | Metric | Mean | Median | Min | Max | StdDev | p95 | | --------------------- | -------- | ------ | ------ | ------ | ------- | -------- | | navigationLoaded | 3641.935 | 3684 | 2979 | 3826 | 183.850 | 3778 | | navigationInteractive | 3658.903 | 3701 | 2996 | 3844 | 183.829 | 3794.950 | | visuallyLoaded | 3940.452 | 3984 | 3233 | 4125 | 181.229 | 4083.250 | | contentInteractive | 3940.710 | 3984 | 3233 | 4125 | 181.299 | 4084.250 | | fullyLoaded | 4077.871 | 4108 | 3321 | 4248 | 186.748 | 4226.300 | | rss | 39.703 | 39.625 | 39.543 | 42.109 | 0.440 | 39.695 | | pss | 22.531 | 22.471 | 22.386 | 24.475 | 0.357 | 22.538 | | uss | 17.693 | 17.645 | 17.559 | 19.305 | 0.296 | 17.711 | patch: | Metric | Mean | Median | Min | Max | StdDev | p95 | | --------------------- | -------- | ------ | ------ | ------ | ------- | -------- | | navigationLoaded | 3558.290 | 3586 | 2829 | 3667 | 141.218 | 3630.650 | | navigationInteractive | 3575.194 | 3603 | 2846 | 3684 | 141.229 | 3646.750 | | visuallyLoaded | 3919.355 | 3951 | 3293 | 4014 | 127.037 | 4010.750 | | contentInteractive | 3919.613 | 3951 | 3293 | 4014 | 127.084 | 4010.750 | | fullyLoaded | 4063.548 | 4089 | 3415 | 4166 | 132.326 | 4160.500 | | uss | 17.906 | 17.898 | 17.801 | 17.992 | 0.050 | 17.973 | | rss | 40.452 | 40.445 | 40.352 | 40.535 | 0.050 | 40.523 | | pss | 23.007 | 23 | 22.903 | 23.093 | 0.050 | 23.075 | I think the results look good. I'm mostly happy about lower stdev.
> I observed in video app similar as noted in gallery group headers missing support for 'run time mirrored' pseudo locale (https://bugzilla.mozilla.org/show_bug.cgi?id=1170973#c7) Yeah, date/time in pseudo will use en-US locale for now. We plan to tackle that in the next release.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: