Closed Bug 1209866 Opened 9 years ago Closed 9 years ago

Move tv_apps from mozL10n.DateTimeFormat to Intl API

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(2 files)

No description provided.
Assignee: nobody → gandalf
Comment on attachment 8669258 [details] [gaia] zbraniecki:1209866-refactor-l10n-in-tvapps > mozilla-b2g:master Hi Evelyn, this is first of two parts of getting tv_apps to use modern l10n API. This one is important to make in for 2.5 because l10n_date is obsolete and I'd like to remove it in 2.5. Can you review this? Thank you!
Attachment #8669258 - Flags: review?(ehung)
Comment on attachment 8669258 [details] [gaia] zbraniecki:1209866-refactor-l10n-in-tvapps > mozilla-b2g:master I did a quick review and it looks good to me except one missing l10 id (commented on the PR). However I'd like to have Rex reviewing the patch as well because he knows more details on the code than me. Unfortunately he is PTO this week, so if you can wait, please get his r+ and land it. Thanks!
Attachment #8669258 - Flags: review?(rexboy)
Attachment #8669258 - Flags: review?(ehung)
Attachment #8669258 - Flags: review+
Comment on attachment 8669258 [details] [gaia] zbraniecki:1209866-refactor-l10n-in-tvapps > mozilla-b2g:master The patch itself looks good to me, but seems dayperiod is not working on my B2G-Desktop. Not sure if it's an issue of mozIntl. Do you have any idea?
Attachment #8669258 - Flags: review?(rexboy) → review+
(But it works good on my device)
Can you share your STR for the B2G Desktop + screenshot?
Flags: needinfo?(rexboy)
Attached image screenshot from b2g-desktop (deleted) —
1.apply the patch 2.make TV build 3.run it on B2G desktop (I use build in Oct. 6). Expected: The clock at upper right shows day period in smaller font before digit. Actual: Day period is shown the same size after digit and thus break the layout. I did a little inspection and saw the dayperiod property was just undefined. Not sure if the root cause is inside Gecko or Gaia.
Flags: needinfo?(rexboy)
I think the issue is unrelated to the patch itself. But since we rely on B2G-Desktop to develop TV apps, we need another bug for resolving it.
Can you share your: 1) navigator.languages 2) f.resolvedOptions() from home.js line 784 3) amPmFmt.resolvedOptions() from home.js line 794 Thanks!
Flags: needinfo?(rexboy)
Also, can you tell me, on this B2G-Desktop, what is the result of those two commands: 1) (new Date()).toLocaleFormat('%p'); 2) (new Date()).toLocaleFormat('%I:%m'); 3) (new Date()).toLocaleString(navigator.languages, {hour12: true, hour: 'numeric', minute: 'numeric'}); 4) (new Date()).toLocaleString(navigator.languages, {hour12: false, hour: 'numeric', minute: 'numeric'}); Sorry for the trouble. I think I know what's going on and I want to verify my hypothesis before I start writing a patch.
Sorry for the late reply. Here they are: navigator.languages: [ "en-US", "en" ] f.resolvedOptions(): Object { hour12: true, hour: "numeric", minute: "numeric" dayperiod: false locale: "en-US" } amPmFmt.resolvedOptions(): Object { hour12: true, dayperiod: true, hour: "numeric", locale: "en-US" } (new Date()).toLocaleFormat('%p'); "下午" (new Date()).toLocaleFormat('%I:%m'); "06:10" (new Date()).toLocaleString(navigator.languages, {hour12: true, hour: 'numeric', minute: 'numeric'}); "6:11 PM" (new Date()).toLocaleString(navigator.languages, {hour12: false, hour: 'numeric', minute: 'numeric'}); "18:11"
Flags: needinfo?(rexboy)
Ok. Thanks! The problem exists because your OS is using zh-CN (I guess), while navigator.languages is using en-US. In that case we receive the system '%p' and try to cut it out of en-US '6:11 PM' which doesn't work. I'm trying to get bug 1216150 to land in time for 2.5 which will enable us to improve the mozIntl heuristics for cross-locale scenarios. I'd like to land this patch anyway in time for 2.5 in order to remove a high number of obsolete entities in bug 1170963. How late would you be ok with landing this patch in 2.5?
Flags: needinfo?(rexboy)
Thank you for specifying the issue. Since it only occurs when OS and navigator.languages are different, it's ok for me to just land this bug if we will fix bug 1216150 for sure.
Flags: needinfo?(rexboy)
Unfortunately I can't be certain of that I will land that patch, but I am certain that I will fix the bug that you are seeing - either with the proper fix or some temporary stitching. I would love to land this because it's the last piece needed to remove l10n_date.js from our tree and remove a lot of very complex and error-prone strings that our localizers have to deal with. That would make 2.5 better. But in order to fix the scenario of different system/product language we need to land a patch that will expose a feature that is not standardized yet. I'm trying to gather consensus behind it from ECMA 402 group. The final decision is up to :waldo and all I can do is provide him with a good patch and a solid proof that the API is not likely to change later. Waldo is on board and we both know that it's not optimal to land it before standardization is complete, but that the feature is really needed for us to avoid many edge cases. All I can say is that TV is not specific here. We have the same problem on all devices and form factors. Launching any of our phones with german locale right now and turning on 12h clock will show the very same bug on lockscreen. My proposal is: 1) Let's land this patch 2) I will try to get the patch from bug 1216150 landed within next two weeks 3) If Waldo's decision will be not to land it, I will have to fix it within mozIntl for all products including TV. I have a backup plan (reuse some of the strings from our localizers) and I will get it fixed within two weeks. Would you be ok with that?
Blocks: 1212151
Depends on: 1216150, 1215068
With fixes in bug 1216150 and bug 1215068 we now should work properly. I tested it and it looks good. :rexboy, can you retest it on your system before I land the patch?
Flags: needinfo?(rexboy)
I'm actually going to land it basing on the r+ I got. I removed the tv-epg pieces from the patch to a separate bug (bug 1247360) and smart-home to bug 1243087. Next I'll migrate smart-system to l20n.js to match what's in apps/system and make merging systems easier. wrt. the desktop simulator issue with prefix, I believe formatToParts solves it, and I'm ok taking a risk and having to fix it post-landing if :rexboy will still be able to reproduce it.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
No longer blocks: 1170963, 1212151
clear needinfo
Flags: needinfo?(rexboy)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: