995940 - [tarako][L10N] No 'data-l10n-id' for ./elements/sim_manager.html: <h2>SIM settings</h2>

Status: RESOLVED FIXED

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

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: screenshot

Steps to reproduce
------------------------------------------------------------------------
1. Set Launguage to Bengali
2. Go to Settings -> SIM Setting
   --> You will see "SIM settings" section title was not translated. Please see attached screenshot.

This is bcz there is no  'data-l10n-id' for ./elements/sim_manager.html: <h2>SIM settings</h2>


Test build
-----------------------------------------------------------------------
Gaia b2802627a974795ccba989cede0540f20fadc633 
Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/c8491a42d4e8 
BuildID 20140413164001 │
Version 28.1 ro.build.version.incremental=eng.cltbld.20140413.210357

Comment 1

[Joe Cheng [:jcheng] (please needinfo)]

triage: let's not block on this but we should uplift when we have a solution

ni? Arthur
ni? l10n for late l10n

Comment 2

[Francesco Lodolo [:flod]]

I'm checking master but I can't find a "SIM Settings" section in Settings. Is this supposed to be displayed only under some conditions?

Comment 3

[Axel Hecht [:Pike]]

FWIW, on branches, we should just re-use the "Settings" string instead of exposing "SIM Settings".

For master, I wonder if that string is the best UX given how often SIM shows up on that screen already.

Comment 4

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

I think it's kinda late to add a new string in v1.3t and not necessary. 

How about use `settings` l10n-id in 1.3t while adding another new string called `simSettings` >= 1.4 ? (in 1.4, 2.0)

Any idea !?

Comment 5

[Axel Hecht [:Pike]]

Sounds good for > 1.4, it's too late for 1.4 as well.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

triage: not  blocking release with this. to backlog

Comment 7

[yang.zhao]

I add the following "string=value" pair in apps/settings/settings.properties for the four languages:
bn-BD
simSettings=SIM সেটিংস

hi-IN
simSettings=SIM सेटिंग्स

ta
simSettings=SIM அமைவுகள்

en-US
simSettings=SIM settings

Meanwhile,we need add 'data-l10n-id="simSettings"' in gaia/apps/settings/elements/sim_manager.html as following：
 <h2 data-l10n-id="simSettings">SIM settings</h2>

James,please help to check it and find a person to uplift it.Thank you!

Comment 8

[James Zhang (Spreadtrum)]

(In reply to yang.zhao from comment #7)
> I add the following "string=value" pair in apps/settings/settings.properties
> for the four languages:
> bn-BD
> simSettings=SIM সেটিংস
> 
> hi-IN
> simSettings=SIM सेटिंग्स
> 
> ta
> simSettings=SIM அமைவுகள்
> 
> en-US
> simSettings=SIM settings
> 
> Meanwhile,we need add 'data-l10n-id="simSettings"' in
> gaia/apps/settings/elements/sim_manager.html as following：
>  <h2 data-l10n-id="simSettings">SIM settings</h2>
> 
> James,please help to check it and find a person to uplift it.Thank you!

Please attach your patch first.

Comment 9

[yang.zhao]

Attachment: simSettings.patch

Comment 10

[yang.zhao]

Only add bn-BD/en-US/ta/hi-IN languages on our branch.---->The patch didn't include these.Please see comment#7

Then need mozilla add this data-l10n-id in gaia.

Comment 11

[Evelyn Hung]

Comment on attachment 8414156 [details] [diff] [review]
simSettings.patch

Hi, thanks for the patch. You also need to add 'simSettings' to settings.en-US.properties.

Comment 12

[Evelyn Hung]

(In reply to yang.zhao from comment #10)
> Only add bn-BD/en-US/ta/hi-IN languages on our branch.---->The patch didn't
> include these.Please see comment#7
> 
> Then need mozilla add this data-l10n-id in gaia.

Please add to settings.en-US.properties to make sure they are consistent.

Comment 13

[James Zhang (Spreadtrum)]

Yang, please update your patch.

Comment 14

[yang.zhao]

(In reply to Evelyn Hung [:evelyn] from comment #11)
> Comment on attachment 8414156 [details] [diff] [review]
> simSettings.patch
> 
> Hi, thanks for the patch. You also need to add 'simSettings' to
> settings.en-US.properties.

Please see comment#7 ,I already added to en-US.

Comment 15

[yang.zhao]

(In reply to James Zhang from comment #13)
> Yang, please update your patch.

I add four languages:bn-BD /hi-IN /ta /en-US
Already including en-US.

Comment 16

[Evelyn Hung]

(In reply to yang.zhao from comment #15)
> (In reply to James Zhang from comment #13)
> > Yang, please update your patch.
> 
> I add four languages:bn-BD /hi-IN /ta /en-US
> Already including en-US.

No, please add one to en-US file in code base, not in your local.

Comment 17

[James Zhang (Spreadtrum)]

(In reply to yang.zhao from comment #15)
> (In reply to James Zhang from comment #13)
> > Yang, please update your patch.
> 
> I add four languages:bn-BD /hi-IN /ta /en-US
> Already including en-US.

git add .
git diff <your last commit> to generate patch.

Comment 18

[yang.zhao]

Attachment: en-US.patch

Comment 19

[yang.zhao]

(In reply to Evelyn Hung [:evelyn] from comment #16)
> (In reply to yang.zhao from comment #15)
> > (In reply to James Zhang from comment #13)
> > > Yang, please update your patch.
> > 
> > I add four languages:bn-BD /hi-IN /ta /en-US
> > Already including en-US.
> 
> No, please add one to en-US file in code base, not in your local.

Ok,please see the attachment en-US.patch

Comment 20

[Joe Cheng [:jcheng] (please needinfo)]

triage: 1.3T+ partner blocker

Comment 21

[Evelyn Hung]

Attachment: bug-995940-patch

This patch is squashed from attachment 8414156 [details] [diff] [review] and 8414326, please merge this one.

Comment 22

[Evelyn Hung]

Hi Yang, I think you can merge this patch to 1.3t by yourself so your name can be kept in commit history.

Comment 23

[thomas tsai]

Hi Yang: please check-in bug-995940-patch to 1.3t.

Comment 24

[Evelyn Hung]

(In reply to thomas tsai from comment #23)
> Hi Yang: please check-in bug-995940-patch to 1.3t.

Hi Yang,
I just help you commit the patch to gaia master with your name
https://github.com/mozilla-b2g/gaia/commit/2ca4964e6d6eacae35c439e84f69e7e48f456f07

Please uplift this commit to 1.3t. Thanks!

Comment 25

[yang.zhao]

(In reply to Evelyn Hung [:evelyn] from comment #24)
> (In reply to thomas tsai from comment #23)
> > Hi Yang: please check-in bug-995940-patch to 1.3t.
> 
> Hi Yang,
> I just help you commit the patch to gaia master with your name
> https://github.com/mozilla-b2g/gaia/commit/
> 2ca4964e6d6eacae35c439e84f69e7e48f456f07
> 
> Please uplift this commit to 1.3t. Thanks!

Already merge it before this comment.
commit:77ba36a191dfc950bc097fcc0f92fb749ac07c2a

Comment 26

[Jason Smith [:jsmith]]

Delphine - Wouldn't we also need this patch on 1.4? There is phones shipping in 1.4 with DSDS support to my understanding.

Comment 27

[Delphine Lebédel [:delphine - Use needinfo]]

Yes, we should get this into 1.4 if it's not too late

Comment 28

[Jason Smith [:jsmith]]

Evelyn - Can you ask for 1.4 approval?

Comment 29

[Evelyn Hung]

Comment on attachment 8414378 [details] [diff] [review]
bug-995940-patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): a lost string id
[User impact] if declined: a string won't be localized 
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): No
[String changes made]: add a l10n-id 'simSettings'

Comment 30

[Francesco Lodolo [:flod]]

(In reply to Delphine Lebédel [:delphine - use needinfo] from comment #27)
> Yes, we should get this into 1.4 if it's not too late

Just to double check: you're aware that this patch breaks string freeze on 1.4, right?

Comment 31

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8414378 [details] [diff] [review]
bug-995940-patch

The patch looks good but let's forward approvals to fabrice :)

Comment 32

[Axel Hecht [:Pike]]

I strongly suggest to not take this for 1.4

Comment 33

[Jason Smith [:jsmith]]

(In reply to Axel Hecht [:Pike] from comment #32)
> I strongly suggest to not take this for 1.4

I disagree on this. DSDS is a requirement for Dolphin & this a front-facing feature in the settings app for DSDS. This has to be translated. Otherwise, we will likely not survive certification.

Comment 34

[Axel Hecht [:Pike]]

See comment 4, we can just work around this string.

Comment 35

[Jason Smith [:jsmith]]

(In reply to Axel Hecht [:Pike] from comment #34)
> See comment 4, we can just work around this string.

Ok - should we open a new bug then to get that implemented for 1.4?

Comment 36

[Axel Hecht [:Pike]]

In other areas of mozilla, it's totally fine to do per-branch patches in one bug. Not really my call.

Comment 37

[James Zhang (Spreadtrum)]

Yes, dolphin project is on going.

Comment 38

[[:fabrice] Fabrice Desré]

Comment on attachment 8414378 [details] [diff] [review]
bug-995940-patch

Review of attachment 8414378 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not doing 1.4 approvals.

Comment 39

[Preeti Raghunath(:Preeti)]

Please provide a different patch that reuses an existing string named settings. l10n seems to be nervous taking this in 1.4

Comment 40

[Delphine Lebédel [:delphine - Use needinfo]]

(In reply to Francesco Lodolo [:flod] from comment #30)
> (In reply to Delphine Lebédel [:delphine - use needinfo] from comment #27)
> > Yes, we should get this into 1.4 if it's not too late
> 
> Just to double check: you're aware that this patch breaks string freeze on
> 1.4, right?

Sorry, I read this bug a bit too quickly and misread Jason's comment :/ 
I agree with Flod and Pike. Let's do per comment 4

Comment 41

[Evelyn Hung]

EJ, can you help this case? Thanks!

Comment 42

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Sure !

Comment 43

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on v1.4

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: users would see unlocalized string on screen
[Testing completed]: no, just add an data-id on the element.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: no

Comment 44

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8426767 [details] [review]
patch on v1.4

Please make the default text consistent to the string specified in the localization file, thanks.

Comment 45

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8426767 [details] [review]
patch on v1.4

Arthur, I just fixed the nits. 

Thanks :)

Comment 46

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8426767 [details] [review]
patch on v1.4

r=me, thanks.

Comment 47

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Cool, thanks Arthur. 

Let's wait for 1.4 approval ! :D

Comment 48

[Preeti Raghunath(:Preeti)]

Comment on attachment 8426767 [details] [review]
patch on v1.4

Taking for 1.4

Comment 49

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks all,

merged the v1.4 patch into Gaia/v1.4 : 60e32bc9334eb21f1ae9b6fef187aab131b837cf