Closed Bug 1020138 Opened 6 years ago Closed 3 years ago

Clean up mozL10n.get uses

Categories

(Firefox OS Graveyard :: Gaia::L10n, defect, P4)

x86
All
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: zbraniecki, Unassigned, Mentored)

References

Details

(Whiteboard: [good first bug][systemsfe][lang=js])

User Story

Migrate use cases of mozL10n.get to use DOM API where possible:

node.setAttribute('data-l10n-id', l10nId) or
navigator.mozL10n.setAttributes(node, l10nId, l10nArgs);

Attachments

(1 file)

After bug 992473 and bug 994519 land, we will be able to significantly reduce the number of use cases of mozL10n.get.

Vast majority of the current uses cases are to manually insert translation into DOM node, which should be done via mozL10n.localize and MO.

Reducing the number of uses of get will help our code to be less prone to retranslation errors.

There are around 400 uses of get in our code. It'll take some time...
Depends on: 992473, 994519
This will be helpful for bug 994357, too.
Blocks: 994357
Depends on: 906580
Priority: -- → P4
Depends on: 1029944
It's (or any chunk of it) a good first bug. It's all about grepping for "mozL10n.get" use cases and if it's simple:

var text = navigator.mozL10n.get(l10nId);
node.textContent = text;

replacing it with:

node.setAttribute('data-l10n-id', l10nId);

And from my research so far, in most cases this is how get is being used.

If you're interested, let me know. It's probably best to isolate one app and file a spin-off bug to work on that.

I'll be happy to mentor.
Mentor: gandalf
Whiteboard: [good first bug]
Depends on: 1030280
Depends on: 1032813
:gandalf, I have an intern starting on July 14th that should be able to work on this. It would be really nice to do this sooner rather than later too since I just fixed a bug where text content wasn't rendered because of poor use of the l10n api. There are probably other areas in our code that do the same and are likely to fail to render. Are you in that week?
Flags: needinfo?(gandalf)
Whiteboard: [good first bug] → [good first bug][systemsfe]
Yeah, I'll be happy to guide/mentor :)
Flags: needinfo?(gandalf)
Depends on: 967925
Depends on: 1038984
Depends on: 1040271
Depends on: 1041403
Depends on: 1041404
Depends on: 1041614
Depends on: 1043615
Depends on: 1035367
User Story: (updated)
Depends on: 1047344
Depends on: 1047347
Depends on: 1052136
Depends on: 1057795
Depends on: 1073893
Stats from branches:

2.0:
apps: 1298
shared: 92

2.1:
apps: 1256
shared: 93

2.2:
apps: 894
shared: 64

Clear improvement in 2.2, but a lot of work remains.
That's a reduction by one third from 2.1 to 2.2. Great work, Zibi!
Yeah, unfortunately many new apps are already picking up mozL10n.get extensively. For example tv_apps already have 137 uses... We need to educate users.
Whiteboard: [good first bug][systemsfe] → [good first bug][systemsfe][lang=js]
Depends on: 1095096
Stats from today for 2.2:

apps: 871 // -23
shared: 53 // -11

spells I use:
 - grep -I -r "[^a-zA-Z0-9_\)]_(" ./apps|wc -l
 - grep -I -r "mozL10n.get[^A]" ./apps|wc -l // to exclude getAttributes
 - grep -I -r "[^z][lL]10n.get" ./apps/|wc -l // LazyL10n, MockL10n and l10n.get()

In reality, I expect the improvement to be better as I added the third command line value which has not been there before.
Depends on: 1100808
Depends on: 1095109
Depends on: 1097827
Depends on: 1115741
I would like to work on this.
Depends on: 1111841
Rohan, it would be best for you to take one of the small pieces first. I'd recommend bug 1111841 or bug 1100808.
Depends on: 1100995
Stats for today:

2.2:

apps: 823 // -48 since comment 8
shared: 61 // +8 since comment 8

master:

apps: 836 // +13 since 2.2
shared: 61 // 0
Attached file get_stats.sh
I developed a dummy bash script to collect stats (works on Mac only I guess).

Here are the stats from today:

2.2:

=========  Apps  ===============
bluetooth: 26
bookmark: 4
calendar: 37
callscreen: 43
camera: 12
clock: 22
collection: 6
communications: 108
costcontrol: 47
email: 34
fl: 18
ftu: 9
gallery: 5
homescreen: 16
music: 46
network-alerts: 1
pdfjs: 29
privacy-panel: 7
ringtones: 6
search: 4
settings: 58
sharedtest: 26
sms: 21
system: 236
verticalhome: 1
video: 10
wappush: 13
================================
Total Apps: 845


========  Shared  ==============
elements: 2
js: 48
pages: 9
test: 2
================================
Total Shared: 61


========  TV Apps  =============
app-deck: 3
smart-home: 6
smart-system: 133
================================
Total TV Apps: 142


master:

=========  Apps  ===============
bluetooth: 40
bookmark: 7
calendar: 40
callscreen: 43
camera: 12
clock: 22
collection: 5
communications: 99
costcontrol: 47
email: 34
fl: 18
ftu: 7
gallery: 5
keyboard: 15
music: 46
network-alerts: 1
pdfjs: 29
privacy-panel: 7
ringtones: 6
search: 4
settings: 58
sharedtest: 26
sms: 21
system: 242
verticalhome: 1
video: 10
================================
Total Apps: 845


========  Shared  ==============
elements: 2
js: 48
pages: 9
test: 2
================================
Total Shared: 2


========  TV Apps  =============
smart-home: 6
smart-system: 29
================================
Total TV Apps: 35


====================================

Some apps went down: wappush to 0!, communications by 11, TV's smart-system reduced a lot. On the other hand bluetooth added 14 and system added 6.
Ok, got a better way to represent the data. Will use it from now on:

+-------------------------+-----+--------+------+
|          Apps           | 2.2 | master | diff |
+-------------------------+-----+--------+------+
| bluetooth               |  26 |     40 |   14 |
| bookmark                |   4 |      7 |    3 |
| calendar                |  37 |     40 |    3 |
| callscreen              |  43 |     43 |    0 |
| camera                  |  12 |     12 |    0 |
| clock                   |  22 |     22 |    0 |
| collection              |   6 |      5 |   -1 |
| communications          | 108 |     99 |   -9 |
| costcontrol             |  47 |     47 |    0 |
| default_theme           |   0 |      0 |    0 |
| download                |   0 |      0 |    0 |
| email                   |  34 |     34 |    0 |
| emergency-call          |   0 |      0 |    0 |
| findmydevice            |   0 |      0 |    0 |
| fl                      |  18 |     18 |    0 |
| fm                      |   0 |      0 |    0 |
| ftu                     |   9 |      7 |   -2 |
| gallery                 |   5 |      5 |    0 |
| homescreen              |  16 |      0 |  -16 |
| keyboard                |   0 |     15 |   15 |
| marketplace.firefox.com |   0 |      0 |    0 |
| music                   |  46 |     46 |    0 |
| network-alerts          |   1 |      1 |    0 |
| operatorvariant         |   0 |      0 |    0 |
| pdfjs                   |  29 |     29 |    0 |
| privacy-panel           |   7 |      7 |    0 |
| ringtones               |   6 |      6 |    0 |
| search                  |   4 |      4 |    0 |
| settings                |  58 |     58 |    0 |
| sharedtest              |  26 |     26 |    0 |
| sms                     |  21 |     21 |    0 |
| system                  | 236 |    242 |    6 |
| verticalhome            |   1 |      1 |    0 |
| video                   |  10 |     10 |    0 |
| wallpaper               |   0 |      0 |    0 |
| wappush                 |  13 |      0 |  -13 |
| Total Apps              | 845 |    845 |    0 |
|                         |     |        |      |
|                         |     |        |      |
| Shared                  |     |        |      |
| elements                |   2 |      2 |    0 |
| js                      |  48 |     48 |    0 |
| locales                 |   0 |      0 |    0 |
| pages                   |   9 |      9 |    0 |
| resources               |   0 |      0 |    0 |
| style                   |   0 |      0 |    0 |
| test                    |   2 |      2 |    0 |
| Total Shared            |  61 |     61 |    0 |
|                         |     |        |      |
|                         |     |        |      |
| TV Apps                 |     |        |      |
| app-deck                |   3 |      0 |   -3 |
| browser                 |   0 |      0 |    0 |
| dashboard               |   0 |      0 |    0 |
| device-deck             |   0 |      0 |    0 |
| dlna-player             |   0 |      0 |    0 |
| fling-player            |   0 |      0 |    0 |
| fling-sender            |   0 |      0 |    0 |
| notification-receiver   |   0 |      0 |    0 |
| notification-sender     |   0 |      0 |    0 |
| smart-home              |   6 |      6 |    0 |
| smart-settings          |   0 |      0 |    0 |
| smart-system            | 133 |     29 | -104 |
| tv-deck                 |   0 |      0 |    0 |
| Total TV Apps           | 142 |     35 | -107 |
+-------------------------+-----+--------+------+
Bug 1159190 landed which should help to bring the numbers down.
Depends on: 1163582
Depends on: 1105648
Depends on: 1169030
Depends on: 1179683
Master from Jul 1st:

=========  Apps  ===============
bluetooth: 40
bookmark: 7
calendar: 40
callscreen: 38
camera: 12
clock: 22
collection: 5
communications: 98
costcontrol: 49
email: 34
fl: 18
ftu: 7
keyboard: 15
music: 46
network-alerts: 1
pdfjs: 29
privacy-panel: 7
ringtones: 6
search: 4
settings: 58
sharedtest: 26
sms: 18
system: 201
verticalhome: 1
video: 10
================================
Total Apps: 792


========  Shared  ==============
elements: 2
js: 48
pages: 9
test: 2
================================
Total Shared: 61


========  TV Apps  =============
dashboard: 1
smart-home: 6
smart-system: 29
tv-epg: 4
================================
Total TV Apps: 40
Depends on: 1187556
Depends on: 1188139
Depends on: 1187668
Depends on: 1104667
Latest numbers from master:

=========  Apps  ===============
bluetooth: 40
bookmark: 7
calendar: 40
camera: 12
clock: 22
collection: 5
communications: 101
costcontrol: 49
email: 34
fl: 18
ftu: 5
keyboard: 15
music: 27
network-alerts: 1
pdfjs: 29
privacy-panel: 7
ringtones: 6
search: 4
settings: 1
sharedtest: 19
system: 196
verticalhome: 1
video: 10
================================
Total Apps: 649


========  Shared  ==============
elements: 2
js: 39
pages: 18
================================
Total Shared: 59


========  TV Apps  =============
smart-home: 6
smart-system: 29
tv-epg: 4
================================
Total TV Apps: 39


SMS, Callscreen and Settings are basically done. Music is going to get new UI soon, next on my list is Communications (bug 1191124) and then I'll basically focus on phasing away l10n_date (bug 1170963).
Depends on: 1191124
Depends on: 1165332
Depends on: 1197019
Depends on: 1197383
Depends on: 1197454
Depends on: 1200078
Depends on: 1206318
Depends on: 1206306
No longer blocks: 1207044
Depends on: 1207044
Depends on: 1208889
Pretty good progress with 2.5. Since we branched, here are the preliminary results:

╔═════════════════════════╦══════╦══════╦══════╗
║                         ║ v2.2 ║ v2.5 ║ diff ║
╠═════════════════════════╬══════╬══════╬══════╣
║ Apps                    ║      ║      ║      ║
║ bluetooth               ║   26 ║   40 ║   14 ║
║ bookmark                ║    4 ║    0 ║   -4 ║
║ calendar                ║   37 ║   21 ║  -16 ║
║ callscreen              ║   43 ║    0 ║  -43 ║
║ camera                  ║   12 ║    0 ║  -12 ║
║ clock                   ║   22 ║    0 ║  -22 ║
║ collection              ║    6 ║    5 ║   -1 ║
║ communications          ║  110 ║  100 ║  -10 ║
║ costcontrol             ║   47 ║   45 ║   -2 ║
║ default_theme           ║    0 ║    0 ║    0 ║
║ download                ║    0 ║    0 ║    0 ║
║ email                   ║   34 ║   34 ║    0 ║
║ emergency-call          ║    0 ║    0 ║    0 ║
║ findmydevice            ║    0 ║    0 ║    0 ║
║ fl                      ║   29 ║   29 ║    0 ║
║ fm                      ║    0 ║    0 ║    0 ║
║ ftu                     ║    9 ║    0 ║   -9 ║
║ gallery                 ║    5 ║    0 ║   -5 ║
║ homescreen              ║   16 ║    1 ║  -15 ║
║ keyboard                ║    0 ║   15 ║   15 ║
║ marketplace.firefox.com ║    0 ║    0 ║    0 ║
║ music                   ║   27 ║    0 ║  -27 ║
║ network-alerts          ║    1 ║    1 ║    0 ║
║ operatorvariant         ║    0 ║    0 ║    0 ║
║ pdfjs                   ║   29 ║   29 ║    0 ║
║ privacy-panel           ║    7 ║    0 ║   -7 ║
║ ringtones               ║    6 ║    6 ║    0 ║
║ search                  ║    4 ║    4 ║    0 ║
║ settings                ║   58 ║    0 ║  -58 ║
║ sharedtest              ║   26 ║   13 ║  -13 ║
║ sms                     ║   21 ║    0 ║  -21 ║
║ sync                    ║    0 ║    0 ║    0 ║
║ system                  ║  215 ║  153 ║  -62 ║
║ verticalhome            ║    1 ║    1 ║    0 ║
║ video                   ║   10 ║    0 ║  -10 ║
║ wallpaper               ║    0 ║    0 ║    0 ║
║ wappush                 ║   13 ║    0 ║  -13 ║
║ Total Apps              ║  818 ║  497 ║ -321 ║
║                         ║      ║      ║      ║
║                         ║      ║      ║      ║
║ Shared                  ║      ║      ║      ║
║ elements                ║    2 ║    2 ║    0 ║
║ js                      ║   48 ║   14 ║  -34 ║
║ locales                 ║    0 ║    0 ║    0 ║
║ pages                   ║    9 ║    0 ║   -9 ║
║ resources               ║    0 ║    0 ║    0 ║
║ style                   ║    0 ║    0 ║    0 ║
║ test                    ║    2 ║    0 ║   -2 ║
║ Total Shared            ║   61 ║   16 ║  -45 ║
║                         ║      ║      ║      ║
║                         ║      ║      ║      ║
║ TV Apps                 ║      ║      ║      ║
║ app-deck                ║    3 ║    0 ║   -3 ║
║ browser                 ║    0 ║   42 ║   42 ║
║ dashboard               ║    0 ║    0 ║    0 ║
║ device-deck             ║    0 ║    0 ║    0 ║
║ dlna-player             ║    0 ║    0 ║    0 ║
║ fling-player            ║    0 ║    0 ║    0 ║
║ fling-sender            ║    0 ║    0 ║    0 ║
║ notification-receiver   ║    0 ║    0 ║    0 ║
║ notification-sender     ║    0 ║    0 ║    0 ║
║ pocket                  ║    0 ║    0 ║    0 ║
║ remote-control          ║    0 ║    0 ║    0 ║
║ remote-control-client   ║    0 ║    0 ║    0 ║
║ smart-home              ║    6 ║    6 ║    0 ║
║ smart-settings          ║    0 ║    0 ║    0 ║
║ smart-system            ║  133 ║   43 ║  -90 ║
║ tv-deck                 ║    0 ║    0 ║    0 ║
║ tv-epg                  ║    0 ║    4 ║    4 ║
║ weather-widget          ║    0 ║    0 ║    0 ║
║ tv_build                ║    0 ║    0 ║    0 ║
║ tv_shared               ║    0 ║    0 ║    0 ║
║ Total TV Apps           ║  142 ║   95 ║  -47 ║
╚═════════════════════════╩══════╩══════╩══════╝
Depends on: 1224063
Depends on: 1243087
Depends on: 1212151
Depends on: 1254169
The current progress for 2.6:

╔═════════════════════════╦══════╦══════╦══════╗
║                         ║ v2.5 ║ v2.6 ║ diff ║
╠═════════════════════════╬══════╬══════╬══════╣
║ Apps                    ║      ║      ║      ║
║ bluetooth               ║   40 ║   25 ║  -15 ║
║ calendar                ║   21 ║   18 ║   -3 ║
║ communications          ║  100 ║   95 ║   -5 ║
║ costcontrol             ║   45 ║   45 ║    0 ║
║ email                   ║   34 ║   26 ║   -8 ║
║ fl                      ║   29 ║   29 ║    0 ║
║ homescreen              ║    1 ║    1 ║    0 ║
║ keyboard                ║   15 ║    0 ║  -15 ║
║ network-alerts          ║    1 ║    0 ║   -1 ║
║ pdfjs                   ║   29 ║   29 ║    0 ║
║ ringtones               ║    6 ║    3 ║   -3 ║
║ search                  ║    4 ║    4 ║    0 ║
║ sharedtest              ║   13 ║   13 ║    0 ║
║ system                  ║  136 ║    0 ║ -136 ║
║ verticalhome            ║    1 ║    1 ║    0 ║
║ Total Apps              ║  475 ║  289 ║ -186 ║
║                         ║      ║      ║      ║
║                         ║      ║      ║      ║
║ Shared                  ║      ║      ║      ║
║ js                      ║   10 ║    0 ║  -10 ║
║ Total Shared            ║   10 ║    0 ║  -10 ║
║                         ║      ║      ║      ║
║                         ║      ║      ║      ║
║ TV Apps                 ║      ║      ║      ║
║ browser                 ║   42 ║    0 ║  -42 ║
║ smart-home              ║    6 ║    0 ║   -6 ║
║ smart-system            ║   51 ║    0 ║  -51 ║
║ tv-epg                  ║    4 ║    0 ║   -4 ║
║ Total TV Apps           ║  103 ║    0 ║ -103 ║
╚═════════════════════════╩══════╩══════╩══════╝
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.