1093267 - Properly separate Dialer/Emergency Call and Callscreen locales files

Status: RESOLVED WONTFIX

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

Description

[Doug Sherk (:drs) (inactive)]

Currently, we're sharing locales files between the Dialer, Emergency Call, and Callscreen apps by directly including locales files from other apps. This is breaking us in certain places such as bug 1070442 and possibly bug 1069989.

This happened because we split the Callscreen app from the Dialer without doing a full audit of every string that was needed.

Ideally, we should have all shared strings in ./shared/locales/dialer/, and single-app strings in ./apps/*/locales/. This bug tracks progress in moving towards that.

Comment 2

[Gabriele Svelto [:gsvelto]]

I can't seem to find the patch I had hacked together but here's a few quick notes I gathered while grep'ing for this issue:

* In shared/locales/keypad/keypad.en-US.properties

** The 'voiceMailKey' is used in apps/emergency-call/test/unit/mock_dialer_index.html, apps/communications/dialer/test/unit/mock_dialer_index.html, apps/communications/dialer/index.html and apps/callscreen/index.html. This causes a warning at build time because the callscreen doesn't include that property file. 

** Similarly apps/communications/dialer/locales/shared.en-US.properties declares a 'voiceMail' property which is used dynamically here:

https://github.com/mozilla-b2g/gaia/blob/d3666f1890866a3cc97521c40c891b3c035d234c/apps/communications/dialer/js/call_info.js#L41

... but not in the HTML code which is still using 'voiceMailKey' as mentioned above.

* In apps/communications/dialer/locales/dialer.en-US.properties

** The 'delete' entry seems to be used only in a mock and has been replaced with 'keypadDelete' from shared/locales/keypad/keypad.en-US.properties in other places

** The following entries seem unused: addNewNumber, confirm-deletion, hideKeypad, reply, ussd-services, hide, call, resume, scClip, scClir, scPwd, serviceClassVoice, serviceClassData, serviceClassFax, serviceClassSms, serviceClassDataSync, serviceClassDataAsync, serviceClassPacket, serviceClassPad, smServiceRegistered, smServiceErased, smServiceNotProvisioned, smClirPermanent, smClirDefaultOnNextCallOn, smClirDefaultOnNextCallOff, smClirDefaultOffNextCallOn, smClirDefaultOffNextCallOff, smPinChanged, smPinUnblocked, smPin2Changed, smPin2Unblocked, emMmiError, emMmiErrorNotSupported, emMmiErrorInvalidAction, emMmiErrorInvalidScCode, emMmiErrorNeedPuk2, emMmiErrorMismatchPin, emMmiErrorInvalidPin, emMmiErrorNeedsPuk, emMmiErrorEnablePin, emMmiErrorMmiFdn, emMmiErrorStkCcUssdDial, emMmiErrorStkCcUssdSs, emMmiErrorStkCcUssdUssd, emMmiErrorStkCcSsDial, emMmiErrorStkCcSsUssd, emMmiErrorStkCcSsSs, emMmiErrorSimBlocked, PasswordIncorrect, SimPin2, SimPuk2, RequestNotSupported, OpNotAllowedDuringVoiceCall, SmsSendFailRetry, SimAbsent, SubscriptionNotAvailable, ModeNotSupported, IllegalSimOrMe, DialModifiedToUssd, DialModifiedToSs, DialModifiedToDial, UssdModifiedToDial, UssdModifiedToSs, UssdModifiedToUssd, SsModifiedToDial, SsModifiedToUssd, SsModifiedToSs, SubscriptionNotSupported, InvalidParameter, RejectedByRemote

** The following entries are used only by the callscreen: connecting, close, incoming, ignore, mute, newCall, outgoing, switch-calls, speakerOption, toggleKeypad, hold, callEnded, via-sim, conferenceCall, caller-left-call, conferenceAddError, conferenceRemoveError, longDateFormat, select-audio-sources, bluetooth-handsfree-device, receiver, speaker

** The following entries are shared between the dialer and callscreen dialerAddContact, keypad, makeCall, merge, 

** All other entries are used only the by comunications/* apps

Note that this should all be verified, especially because I don't know where all of those MMI-related strings are gone (where they used at some point? Why were they removed and when?).

Also I've used this super-crude script to help me out. It should catch almost all occurrences but tweak it to your liking (also I'm not sure how to deal with the .ariaLabel annotations so I've ignored them).

cat apps/communications/dialer/locales/dialer.en-US.properties | grep -v ^# | cut -d'=' -f1 | cut -d'.' -f1 | while read entry; do if [ -n "$entry" ]; then printf "\n$entry matched instances\n"; grep -r "\"$entry\"" shared/ apps/communications/ apps/callscreen/ apps/emergency-call/ ; grep -r "'$entry'" shared/ apps/communications/ apps/callscreen/ apps/emergency-call/ ; fi ; done | less

Comment 3

[Gabriele Svelto [:gsvelto]]

BTW, this bug got me thinking. We should always have a way to identify which strings are in use and which are not. For the ones statically stored in the HTML code this is easy but for the ones used dynamically in JS code it's impossible, generally speaking. So the only way to ensure that we don't miss anything is to always require unit-tests covering the dynamic use of localization strings in JS code. This way removing or renaming a string will always cause a test to fail.

Comment 4

[Staś Małolepszy :stas]

Gabriele: yeah, that would be very helpful indeed.  I've almost finished working on bug 928740 which will report (by breaking builds) strings used in HTML which are not defined in .properties.  It would be hard to report obsolete strings in the same fashion, b/c we don't know which of those not used in HTML could be used by the JS code.

A long-term solution could be to recommend to have two files: static.en-US.properties and dynamic.en-US.properties which would only contain one specific kind of strings; to be used in HTML and to be used in JS, respectively.  Zibi, any other ideas?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Staś Małolepszy :stas from comment #4) 
> A long-term solution could be to recommend to have two files:
> static.en-US.properties and dynamic.en-US.properties which would only
> contain one specific kind of strings; to be used in HTML and to be used in
> JS, respectively.  Zibi, any other ideas?

Nope, and I'm not sure how much would that help.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Hi! any progress here? 

Build system will have a refactoring work soon, and hope that any guys can help us remove this workaround. thanks. :)

Comment 7

[Gabriele Svelto [:gsvelto]]

(In reply to Ricky Chien [:rickychien] from comment #6)
> Hi! any progress here? 

Unfortunately no, since this isn't a blocker it kind of stalled.