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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(1 file)
Mostly remove mozL10n.get and l10n_date to Intl
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 2•9 years ago
|
||
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)
Assignee | ||
Comment 3•9 years ago
|
||
: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.
Comment 4•9 years ago
|
||
Comment on attachment 8651323 [details]
pull request
Transferring the review to Punam.
Attachment #8651323 -
Flags: review?(dflanagan) → review?(pdahiya)
Comment 5•9 years ago
|
||
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+
Assignee | ||
Comment 6•9 years ago
|
||
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.
Assignee | ||
Comment 7•9 years ago
|
||
> 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.
Assignee | ||
Comment 8•9 years ago
|
||
Thanks for the review!
Commit: https://github.com/mozilla-b2g/gaia/commit/e4b0d234a8aec8fd58dd836f68db0323b503bd92
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.
Description
•