The default bug view has changed. See this FAQ.

Clean up mozL10n.get uses

NEW
Unassigned

Status

Firefox OS
Gaia::L10n
P4
normal
3 years ago
a year ago

People

(Reporter: gandalf, Unassigned, Mentored)

Tracking

(Depends on: 3 bugs)

Firefox Tracking Flags

(Not tracked)

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 attachment)

(Reporter)

Description

3 years ago
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...
(Reporter)

Updated

3 years ago
Depends on: 992473, 994519
This will be helpful for bug 994357, too.
Blocks: 994357
(Reporter)

Updated

3 years ago
Depends on: 906580
(Reporter)

Updated

3 years ago
Priority: -- → P4
(Reporter)

Updated

3 years ago
Depends on: 1029944
(Reporter)

Comment 2

3 years ago
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@aviary.pl
Whiteboard: [good first bug]
(Reporter)

Updated

3 years ago
Depends on: 1030280
Depends on: 1032813

Comment 3

3 years ago
: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]
(Reporter)

Comment 4

3 years ago
Yeah, I'll be happy to guide/mentor :)
Flags: needinfo?(gandalf)
(Reporter)

Updated

3 years ago
Depends on: 967925

Updated

3 years ago
Depends on: 1038984

Updated

3 years ago
Depends on: 1040271
(Reporter)

Updated

3 years ago
Depends on: 1041403
(Reporter)

Updated

3 years ago
Depends on: 1041404
(Reporter)

Updated

3 years ago
Depends on: 1041614
(Reporter)

Updated

3 years ago
Depends on: 1043615
(Reporter)

Updated

3 years ago
Depends on: 1035367
(Reporter)

Updated

3 years ago
User Story: (updated)
(Reporter)

Updated

3 years ago
Depends on: 1047344
(Reporter)

Updated

3 years ago
Depends on: 1047347
(Reporter)

Updated

3 years ago
Depends on: 1052136
(Reporter)

Updated

3 years ago
Depends on: 1057795
(Reporter)

Updated

3 years ago
Depends on: 1073893
(Reporter)

Comment 5

2 years ago
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!
(Reporter)

Comment 7

2 years ago
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]
(Reporter)

Updated

2 years ago
Depends on: 1095096
(Reporter)

Comment 8

2 years ago
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.
(Reporter)

Updated

2 years ago
Depends on: 1100808
(Reporter)

Updated

2 years ago
Depends on: 1095109
(Reporter)

Updated

2 years ago
Depends on: 1097827
(Reporter)

Updated

2 years ago
Depends on: 1115741

Comment 9

2 years ago
I would like to work on this.
(Reporter)

Updated

2 years ago
Depends on: 1111841
(Reporter)

Comment 10

2 years ago
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: 1121814
Depends on: 1121815
Depends on: 1121816
Depends on: 1100995
(Reporter)

Comment 11

2 years ago
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
(Reporter)

Comment 12

2 years ago
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.
(Reporter)

Comment 13

2 years ago
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.
(Reporter)

Updated

2 years ago
Depends on: 1163582
(Reporter)

Updated

2 years ago
Depends on: 1105648
(Reporter)

Updated

2 years ago
Depends on: 1169030
(Reporter)

Updated

2 years ago
Depends on: 1179683
(Reporter)

Comment 15

2 years ago
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
(Reporter)

Updated

2 years ago
Depends on: 1187556
(Reporter)

Updated

2 years ago
Depends on: 1188139
(Reporter)

Updated

2 years ago
Depends on: 1187668
(Reporter)

Updated

2 years ago
Depends on: 1104667
(Reporter)

Comment 16

2 years ago
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).
(Reporter)

Updated

2 years ago
Depends on: 1191124
(Reporter)

Updated

2 years ago
Depends on: 1165332
(Reporter)

Updated

2 years ago
Depends on: 1197019
(Reporter)

Updated

2 years ago
Depends on: 1197383
(Reporter)

Updated

2 years ago
Depends on: 1197454
(Reporter)

Updated

2 years ago
Depends on: 1200078
(Reporter)

Updated

2 years ago
Depends on: 1206318
(Reporter)

Updated

2 years ago
Depends on: 1206306
(Reporter)

Updated

2 years ago
Blocks: 1207044
(Reporter)

Updated

2 years ago
No longer blocks: 1207044
Depends on: 1207044
(Reporter)

Updated

2 years ago
Depends on: 1208889
(Reporter)

Comment 17

a year ago
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 ║
╚═════════════════════════╩══════╩══════╩══════╝
(Reporter)

Updated

a year ago
Depends on: 1224063
(Reporter)

Updated

a year ago
Depends on: 1243087
(Reporter)

Updated

a year ago
Depends on: 1212151
(Reporter)

Updated

a year ago
Depends on: 1254169
(Reporter)

Comment 18

a year ago
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 ║
╚═════════════════════════╩══════╩══════╩══════╝
You need to log in before you can comment on or make changes to this bug.