Last Comment Bug 1020138 - Clean up mozL10n.get uses
: Clean up mozL10n.get uses
Status: NEW
[good first bug][systemsfe][lang=js]
:
Product: Firefox OS
Classification: Client Software
Component: Gaia::L10n (show other bugs)
: unspecified
: x86 All
P4 normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors: Zibi Braniecki [:gandalf][:zibi]
Depends on: 967925 1095096 1097827 906580 992473 994519 1029944 1030280 1032813 1035367 1038984 1040271 1041403 1041404 1041614 1043615 1047344 1047347 1052136 1057795 1073893 1095109 1100808 1100995 1104667 1105648 1111841 1115741 1121814 1121815 1121816 1163582 1165332 1169030 1179683 1187556 1187668 1188139 1191124 1197019 1197383 1197454 1200078 1206306 1206318 1207044 1208889 1212151 1224063 1243087 1254169
Blocks: 994357
  Show dependency treegraph
 
Reported: 2014-06-04 00:40 PDT by Zibi Braniecki [:gandalf][:zibi]
Modified: 2016-03-07 16:32 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
get_stats.sh (1.99 KB, text/plain)
2015-05-07 14:56 PDT, Zibi Braniecki [:gandalf][:zibi]
no flags Details

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);      
Description User image Zibi Braniecki [:gandalf][:zibi] 2014-06-04 00:40:59 PDT
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...
Comment 1 User image Staś Małolepszy :stas 2014-06-04 01:44:36 PDT
This will be helpful for bug 994357, too.
Comment 2 User image Zibi Braniecki [:gandalf][:zibi] 2014-06-24 19:45:09 PDT
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.
Comment 3 User image Aus Lacroix [:aus] 2014-07-10 13:44:16 PDT
: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?
Comment 4 User image Zibi Braniecki [:gandalf][:zibi] 2014-07-10 13:58:29 PDT
Yeah, I'll be happy to guide/mentor :)
Comment 5 User image Zibi Braniecki [:gandalf][:zibi] 2014-10-27 16:07:47 PDT
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.
Comment 6 User image Staś Małolepszy :stas 2014-10-28 02:52:55 PDT
That's a reduction by one third from 2.1 to 2.2. Great work, Zibi!
Comment 7 User image Zibi Braniecki [:gandalf][:zibi] 2014-10-28 08:27:31 PDT
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.
Comment 8 User image Zibi Braniecki [:gandalf][:zibi] 2014-11-26 15:58:18 PST
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.
Comment 9 User image Rohan Rawat 2014-12-30 01:20:11 PST
I would like to work on this.
Comment 10 User image Zibi Braniecki [:gandalf][:zibi] 2014-12-30 21:23:56 PST
Rohan, it would be best for you to take one of the small pieces first. I'd recommend bug 1111841 or bug 1100808.
Comment 11 User image Zibi Braniecki [:gandalf][:zibi] 2015-04-27 15:29:15 PDT
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
Comment 12 User image Zibi Braniecki [:gandalf][:zibi] 2015-05-07 14:56:52 PDT
Created attachment 8603026 [details]
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.
Comment 13 User image Zibi Braniecki [:gandalf][:zibi] 2015-05-07 15:19:17 PDT
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 |
+-------------------------+-----+--------+------+
Comment 14 User image Staś Małolepszy :stas 2015-05-10 06:38:47 PDT
Bug 1159190 landed which should help to bring the numbers down.
Comment 15 User image Zibi Braniecki [:gandalf][:zibi] 2015-07-02 11:06:34 PDT
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
Comment 16 User image Zibi Braniecki [:gandalf][:zibi] 2015-08-09 01:09:16 PDT
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).
Comment 17 User image Zibi Braniecki [:gandalf][:zibi] 2015-11-03 09:34:39 PST
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 ║
╚═════════════════════════╩══════╩══════╩══════╝
Comment 18 User image Zibi Braniecki [:gandalf][:zibi] 2016-03-07 16:32:55 PST
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 ║
╚═════════════════════════╩══════╩══════╩══════╝

Note You need to log in before you can comment on or make changes to this bug.