We have a small number of mozL10n.get calls remaining in ./shared. It would be good to remove them to simplify the porting of apps to l20n.

Comment on attachment 8670568 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared2 > mozilla-b2g:master Alberto, can you look at this? System still has a lot of mozL10n.get calls. With bug 1041252, bug 1073893 and bug 1163582 I got it down from ~200 in 2.2 to ~150 in 2.5. This will remove it from the last system related file in shared/js. Post 2.5 branching I'll try to remove the rest.

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Gabriele, can you review the comms part for me?

Kevin, the last file in ./shared that has mozL10n.get and I'm trying to move away from it seems to be created by you [0]. I'm trying to trace down where it's used and see if I can just move it to data-l10n-id, or do I need formatValue, but it's not easily greppable, so I'm struggling. I know that this code, or pieces of it are used in ./apps/collection, ./apps/search and ./apps/verticalhome. VrticalHome is supposed to be replaced with new Homescreen if I'm correct, Search doesn't seem to use GaiaGrid.name (or I can't find it), and Collection doesn't seem to use GaiaGrid from elements either (reading the code I feel like they duplicate a lot of the code, but not pick from the file in shared). Can you shed some light on it? How can I move the name getter [1] to either return a Promise or id/args for data-l10n-id/args? [0] https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/collection.js [1] https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/collection.js#L56-L63

Comment on attachment 8670568 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared2 > mozilla-b2g:master Looks good. Thanks for fixing this!

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Mostly looks good but I'd like to see two small changes which I've described in the PR but I'll summarize here: - Always use mozL10n.setAttributes() instead of setting attributes manually for consistency (unless there's a drawback I'm not aware of) - Make Utils.getLocalizedPhoneNumberAdditionalInfo() always return an object with an id/args couple or just an id field. This way we can avoid checking for the return value and just set the attributes on the node directly. BTW, since we're getting rid of Utils.getPhoneNumberAdditionalInfo() we might just rename Utils.getLocalizedPhoneNumberAdditionalInfo() to Utils.getPhoneNumberAdditionalInfo() so as to make it less verbose than it already is. I'm adding Francisco as a reviewer for the contacts bits.

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Oops, forgot to clear my r? flag, please set it again once you've updated the PR.

Commit: https://github.com/mozilla-b2g/gaia/commit/ec556fc41b45a6342c051b8ed8d2ee054e8ea17c

I'd like to ask for QA of the ICC System area. I don't fully understand how to test it, so also NI'ing Bevis for help with giving QA steps to test my ICC changes.

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5) > Kevin, the last file in ./shared that has mozL10n.get and I'm trying to move > away from it seems to be created by you [0]. > > I'm trying to trace down where it's used and see if I can just move it to > data-l10n-id, or do I need formatValue, but it's not easily greppable, so > I'm struggling. You will probably need to use formatValue in this case if we want to keep it. > VrticalHome is supposed to be replaced with new Homescreen if I'm correct, > Search doesn't seem to use GaiaGrid.name (or I can't find it), and > Collection doesn't seem to use GaiaGrid from elements either (reading the > code I feel like they duplicate a lot of the code, but not pick from the > file in shared). > > Can you shed some light on it? How can I move the name getter [1] to either > return a Promise or id/args for data-l10n-id/args? Turning this into a promise might be quite tricky - I think we'd need to do a bit of refactoring for the grid logic. Is doing this one of your 2.5 goals? One thing which we are doing in 2.5 is removing collections. If we ship the new home screen we can likely just remove this code. Regardless we can probably delete collection functionality and get rid of this code. This is something that I can probably do, when do you need it by?

It would be great to have it in 2.5, especially if that means we can remove associated strings and lower the burden on localizers, but it's not blocking me, so anywhere before branching would be awesome. Thanks Kevin!

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Contacts part looking good. Thanks!

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Almost there but there's still an issue I hadn't noticed before. First of all there's a small change that's missing in the code, I've commented on it on the PR. Then there's a problem with the phone_type_* and the phone_type_*_and_carrier strings. Those are declared in apps/callscreen/locales/callscreen*.properties but that file is obviously included only by the callscreen. With your patch applied those strings are also used in the call log but they're missing there as the call log lives in the dialer app. So those strings should be included in both apps, possibly by moving them into shared/locales/telephony/telephony*.properties which is already included in both apps or another shared file if you prefer.

Comment on attachment 8670568 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared2 > mozilla-b2g:master Thanks a lot Gabriele! You're totally right that I forgot to add the strings. I fixed that now and I also noticed that outgoing/incoming strings were not used, so I switched Callscreen to use them. Let me know what you think :)

Comment on attachment 8670568 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared2 > mozilla-b2g:master I think you set the flag on the wrong attachment, I'll have a look at the other PR ;)

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #15) > Thanks a lot Gabriele! You're totally right that I forgot to add the > strings. I fixed that now and I also noticed that outgoing/incoming strings > were not used, so I switched Callscreen to use them. I've just had a look at your latest patch and I'd love to r+ it but I just can't due to the outgoing/incoming string changes. The elements we're setting them on are not leaf elements so setting data-l10n-id instead of aria-label makes all its contents disappear completely breaking the callscreen UI. It's an annoying old problem which I'd like to see fixed but I don't think this bug is the right place to do it. Anyway you'll have to revert these changes to make the patch work: https://github.com/mozilla-b2g/gaia/pull/32281/files#diff-0cc741478a6ca5d2f75dd44f7f7ab18bL368 https://github.com/mozilla-b2g/gaia/pull/32281/files#diff-0cc741478a6ca5d2f75dd44f7f7ab18bL372 BTW do you have a device to test these changes? If you don't I can help you set up the emulator, it can be used for emulating calls and stuff in a far more convenient way than manually testing on a phone.

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Ok, the problem with the strings was due to entity ID overlap between dialer and callscreen. That's fixed now, so rerequesting review :)

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master I think you forgot pushing to the PR because I can't see the new incoming/outgoing labels.

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master Uh, yeah, I guess I forgot to push my latest change. Now it's in, sorry for the trouble!

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #10) > I'd like to ask for QA of the ICC System area. I don't fully understand how > to test it, so also NI'ing Bevis for help with giving QA steps to test my > ICC changes. Sorry, I am more familiar with the gecko part instead. For the icc/icc_worker.js it's more related to the STK menu of the ICC. You shall be able to test it with the ICC in which the STK is available by accessing the STK menu from Settings -> Operation Services. (The last category at the bottom of the Settings menu if STK is available in the inserted ICC.) You can go through all the menu items to see if all the menu items, dialogs are displayed properly.

Comment on attachment 8670559 [details] [gaia] zbraniecki:1212151-remove-mozl10n-get-from-shared > mozilla-b2g:master LGTM, ship it! http://shipitsquirrel.github.io/

Thanks! Commit: https://github.com/mozilla-b2g/gaia/commit/2b3a34cf03ebf9009c368fb3c49af42d6c031bd8 :kgrandon - do you want me to keep this bug open until you have a chance to land bug 1213172?

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #23) > Thanks! > > Commit: > https://github.com/mozilla-b2g/gaia/commit/ > 2b3a34cf03ebf9009c368fb3c49af42d6c031bd8 > > :kgrandon - do you want me to keep this bug open until you have a chance to > land bug 1213172? Up to you. Probably not necessary to keep open if you've done all the necessary work you need to, without 1213172.

Should this bug be marked as fixed?

We're waiting for Kevin in bug 1213172. Kevin, do you have ETA for that one? We're getting close to branching.

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #26) > We're waiting for Kevin in bug 1213172. Kevin, do you have ETA for that one? > We're getting close to branching. That code is dead now, so should be soon. Likely this week.

We're now just waiting for bug 1216150 to land to remove l10n_date.js

There are no more mozL10n.get calls in ./shared.