1043662 - [Flame] Missing labels in Firefox Ringtones and Alerts

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

Attachment: 2014-07-24-19-04-11.png

[Blocking Requested - why for this release]: Nominating because this is a highly visible UI regression.

Flame, while running:

Gaia   29266e18c35f4e72e35f1bba0e34f2fb6b995cc3
SourceStamp 178fe2efc41d
BuildID 20140724000201
Version 32.0
Base: v122


STR:
1. Open Settings | Sound | Ringer
2. Toggle the Ringer widget to open the Select Sound window (currently in the "Change" state)
3. Observe the attached screenshot

Missing Sound: Firefox OS
Missing Alert: ?

Comment 1

[Pi Wei Cheng [:piwei] (inactive)]

We have to branch check first.

Comment 2

[Pi Wei Cheng [:piwei] (inactive)]

Issue DOES occur on Flame 2.0 and Buri 2.0.

Observed: When accessing 'Select Sound' page for changing Ringer sound, the default Firefox OS sound label is displayed as blank. This only occurs to Firefox OS sound label.

Device: Flame
Build ID: 20140724190006
Gaia: 9b6d7357031f2412b18a2fb140d5c974842d4393
Gecko: fbb3b8be8f6c
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Buri
Build ID: 20140724190006
Gaia: 9b6d7357031f2412b18a2fb140d5c974842d4393
Gecko: fbb3b8be8f6c
Version: 32.0 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

-------

Issue does NOT occur on Flame 2.1 master and Flame 1.4.

Observed: All sound labels display when accessing Select Sound page for changing device Ringer.

Device: Flame
Build ID: 20140725034810
Gaia: 3a06aa58245eaf848242d6d1497c1af536fffabd
Gecko: 6c0971104909
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame
Build ID: 20140724180206
Gaia: 6e4eaa5befce9c1812e07bcc78b2138645bdbe7a
Gecko: 300e058993ca
Version: 30.0 (1.4)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 3

[Joshua Mitchell (Inactive)]

QA-wanted: We'll be looking for a Broken / Fixed window somewhere between 2.0 and 2.1

Comment 5

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1031430 seems to be the cause of this issue.

B2g-32 Regression Window:

Last Working 
Environmental Variables:
Device: Flame 2.0
BuildID: 20140722122057
Gaia: 372b58dff18d1493ea7eb05ade75085545625f3e
Gecko: 867ddf8b78f7
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

First Broken 
Environmental Variables:
Device: Flame 2.0
BuildID: 20140722125155
Gaia: 2f882a831d678826ea2ccb071ec38be2b1c12d77
Gecko: ee111c1ded88
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0



Last working gaia / First broken gecko - Issue does NOT occur
Gaia: 372b58dff18d1493ea7eb05ade75085545625f3e
Gecko: ee111c1ded88

First broken gaia / Last working gekko - Issue DOES occur
Gaia: 2f882a831d678826ea2ccb071ec38be2b1c12d77
Gecko: 867ddf8b78f7

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/372b58dff18d1493ea7eb05ade75085545625f3e...2f882a831d678826ea2ccb071ec38be2b1c12d77

Comment 6

[Joshua Mitchell (Inactive)]

broken by bug 1031430, can you take a look Jim?

Comment 8

[Jim Porter (:squib)]

I just updated to the latest gaia/v2.0[1] and this works fine for me, both when flashing with `make production` and `make reset-gaia` (and with MOZILLA_OFFICIAL on and off).

However, for some reason it *doesn't* work if you use the B2G flashing tools[2]. This looks more like a build system issue, which really isn't my area of expertise.

[1] https://github.com/mozilla-b2g/gaia/commit/90a5a08192586f3e91852053e76fbb8321273f9d
[2] https://github.com/Mozilla-TWQA/B2G-flash-tool

Comment 9

[Hema Koka [:hema]]

Thomas, Could you please help with this? From comment 8, it looks like this is something to do with the flashing tools.

Also I am not sure which component this needs to go under -- perhaps gaia::build? Please correct the component. 

Thanks
Hema

Comment 10

[thomas tsai]

Hi Viral, please help.

Comment 11

[viral [:viralwang]]

Hi CatLee,

I'm not sure what's the difference between PVT build and my local build, it looks this bug can only reproduce in PVT build but not in my local build.

here are my steps:
1) make sure we use latest gaia commit on PVT: 2e85678de2c8e13e585288d4cec7d6673cee17ee
2) flash flame by PVT build
   => issue happened.
3) use my local build (make reset-gaia) by same commit(2e85678de2c8e13e585288d4cec7d6673cee17ee)
   => issue NOT happened.

Could you please have some comments on this? Thanks!

Comment 12

[Chris AtLee [:catlee]]

Perhaps it has to do with all the additional languages that are packaged with our builds? We also run with GAIA_OPTIMIZE=1 set.

Comment 13

[Jim Porter (:squib)]

`production` implies `GAIA_OPTIMIZE=1`, and i tested `production`, so that's probably not the problem.

Comment 14

[Salvador de la Puente González [:salva]]

Can I test this without a LDAP account?

Comment 15

[Marcia Knous [:marcia]]

(In reply to Salvador de la Puente González [:salva] from comment #14)
> Can I test this without a LDAP account?

Yes, by following the directions here: https://developer.mozilla.org/en-US/Firefox_OS/Developer_phone_guide/Flame#Updating_your_Flame_to_a_nightly_build

Comment 16

[Salvador de la Puente González [:salva]]

(In reply to Marcia Knous [:marcia - use needinfo] from comment #15)
> (In reply to Salvador de la Puente González [:salva] from comment #14)
> > Can I test this without a LDAP account?
> 
> Yes, by following the directions here:
> https://developer.mozilla.org/en-US/Firefox_OS/Developer_phone_guide/
> Flame#Updating_your_Flame_to_a_nightly_build

Following the steps indicated there, the issues its reproducible.

Comment 17

[Salvador de la Puente González [:salva]]

It seems |ringer_firefox| is missed in |gaia.zip| l10n files for ringtones application. How is this zip generated? Because a zipped version of |PRODUCTION=1 make| inside a Gaia's git clone at the given revision is working perfectly.

Comment 18

[Marcia Knous [:marcia]]

(In reply to Salvador de la Puente González [:salva] from comment #17)
> It seems |ringer_firefox| is missed in |gaia.zip| l10n files for ringtones
> application. How is this zip generated? Because a zipped version of
> |PRODUCTION=1 make| inside a Gaia's git clone at the given revision is
> working perfectly.

Salvador: I am not really sure, probably a better question for someone in dev?

Comment 19

[Salvador de la Puente González [:salva]]

Sorry, I though you were from dev. I repeat my point: 

It seems |ringer_firefox| is missed in |gaia.zip| l10n files for ringtones application. How is this zip generated? Because a zipped version of |PRODUCTION=1 make| inside a Gaia's git clone at the given revision is working perfectly.

:askeing, I saw you're contributing with the B2G-flash-tool, do you know who is in charge of creating the nightly zipped versions of Gaia and how are they created?

Comment 20

[Julien Wajsberg [:julienw]]

Salva, are you sure you checked in your file? I don't find any properties file called ringer_firefox in Gaia (I can only find the opus file).

Comment 21

[Julien Wajsberg [:julienw]]

Comment 20 was some noise, forget it.

PVTBuilds uses multilocale builds (see [1]). I wonder if /locales is not thrown away by multilocale builds, in that case we'd have a similar issue in SMS.

[1] https://developer.mozilla.org/en-US/Firefox_OS/Building#Building_multilocale

Comment 22

[Salvador de la Puente González [:salva]]

I launched a couple of builds against my v2.0 branch with:

> make clean && LOCALES_BASEDIR=locales/ LOCALES_FILE=locales/languages_own.json GAIA_DEFAULT_LOCALE=es make

> make clean && LOCALES_BASEDIR=locales/ LOCALES_FILE=locales/languages_own.json GAIA_DEFAULT_LOCALE=es PRODUCTION=1 MOZILLA_OFFICIAL=1 make

> make clean && LOCALES_BASEDIR=locales/ LOCALES_FILE=locales/languages_own.json GAIA_DEFAULT_LOCALE=es PRODUCTION=1 make

The only language is Spanish and there is no problem when zipping the profile neither. Can we know how exactly are these zips being built?

Comment 23

[Julien Wajsberg [:julienw]]

I think you need to set LOCALES_BASEDIR to the directory that comes from gaia-l10n repository, if you follow correctly the "building multilocale" documentation.

Comment 24

[Salvador de la Puente González [:salva]]

But I download the repositories to locales/ as the tutorial say:

> Clone the appropriate locales from http://hg.mozilla.org/gaia-l10n into a directory; we use gaia-l10n/. You could use the locales/ directory . You'll need to clone a repo for each locale listed in the languages file.

I will try again clonning it into another folder, what could go wrong?

Comment 25

[Salvador de la Puente González [:salva]]

Wow. A STR at least:

 1. Make dir inside gaia called gaia-l10n
 2. Clone all the repositories from http://hg.mozilla.org/gaia-l10n/ into that directory. You can execute (inside Gaia folder):

> for l in `egrep -ho "\"(\w)+\"\s+:" ./locales/languages_all.json | tr -d " \":" | tr \\n " "`
hg clone "http://hg.mozilla.org/gaia-l10n/$l" "gaia-l10n/$l"

 3. Export the following environment variables:

> export LOCALE_BASEDIR=$PWD/gaia-l10n
> export LOCALES_FILE=$PWD/locales/languages_all.json
> export GAIA_DEFAULT_LOCALE=en-US

 4. Execute make clean && make-profile
 5. Make a dir called gaia, move the profile directory inside and zip all
 6. Use shallow_flash.sh script with that zipped gaia

As you can notice, yo won't need to make production, simply a multilocale build and the labels for ringer and notifications will be missed.

Comment 26

[Francesco Lodolo [:flod]]

Can you try without setting GAIA_DEFAULT_LOCALE?

Comment 27

[Salvador de la Puente González [:salva]]

It seems that making a multilocale build suffices for the labels to be missed. Cloned locales don't include branding names for ringer and notifiers. I'm gonna see if we can uplift build changes from master to 2.0 in order to fix this.

Yuren, do you have any clue about this?

Comment 28

[Salvador de la Puente González [:salva]]

(In reply to Francesco Lodolo [:flod] from comment #26)
> Can you try without setting GAIA_DEFAULT_LOCALE?

Yep, I tried only setting LOCALE_BASEDIR and the problem reproduces.

Comment 29

[Francesco Lodolo [:flod]]

(In reply to Salvador de la Puente González [:salva] from comment #27)
> It seems that making a multilocale build suffices for the labels to be
> missed. Cloned locales don't include branding names for ringer and
> notifiers. I'm gonna see if we can uplift build changes from master to 2.0
> in order to fix this.

If you work on 2.0, you may want to use the right l10n repositories
http://hg.mozilla.org/releases/gaia-l10n/v2_0/

gaia-l10n is following master these days.

It's expected not to have that string on 2.0 in the repo: 2.0 is string frozen, we load sounds-hacky-2.0.properties to avoid breaking string freeze. 

2.1 has the string, and so do locales up to date
https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/sounds/sounds.en-US.properties

Comment 30

[Salvador de la Puente González [:salva]]

Thank you Francesco for your feedback. Even with the proper repositories, the issue reproduces. I think it is because of the build procedure.

Comment 31

[Julien Wajsberg [:julienw]]

Salva, can you try loading the hacky properties file from another directory, eg locales-hacky?

Comment 32

[Salvador de la Puente González [:salva]]

It seems the problem is related with bug 1031430 (v2.0 patch) that does not take into account how multilocale script works so *-hacky-2.0.properties files don't get copied into build_stage and the build procedure is logging they can not be found:

> /home/salva/workspace/gaia/build_stage/sms/locales/sms-hacky-2.0.properties could not be found.
> /home/salva/workspace/gaia/build_stage/ringtones/locales/sounds-hacky-2.0.properties could not be found.

We have two options. I can modify the multilocale script to copy *-hacky-2.0.properties, or we can uplift some strings related diffs to v2.0 [1]

Comment 33

[Salvador de la Puente González [:salva]]

Sorry, [1] is https://github.com/mozilla-b2g/gaia/commit/3d2fff53d3aaa53bb41bc9ebe9a5611476d330a7#diff-60358ee1d8c7b1b52637351790957d96L15

Comment 34

[Francesco Lodolo [:flod]]

(In reply to Salvador de la Puente González [:salva] from comment #32)
> We have two options. I can modify the multilocale script to copy
> *-hacky-2.0.properties, or we can uplift some strings related diffs to v2.0
> [1]

Only option 1 is viable for 2.0.

Comment 35

[Julien Wajsberg [:julienw]]

This issue is affecting the SMS app as well: because of this issue we don't see the SIM number on the send button in DSDS devices with 2 SIMs in v2.0 multilocale builds.

Comment 36

[Salvador de la Puente González [:salva]]

Attachment: For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

Hello Yuren. I was talking with Julien about this bug which caused the SMS application to loose the SIM number label in the send button. We think we could follow another strategy for non-localizable strings. Julian was suggesting something like separated locales, I was suggesting to annotate some way non localizable keys.

Maybe there is a better way to include hacky files such as explicitly including them into the default section of the *.ini files but this quick and dirty patch solves the problem for 2.0. What do you think?

Comment 37

[Salvador de la Puente González [:salva]]

Attachment: Ringer selection after patch

Comment 38

[Salvador de la Puente González [:salva]]

Attachment: Send button of SMS application after patch

Comment 39

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8472463 [details] [review]
For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

let's take this workaround, r=yurenju if add comment to explain why we need this workaround.

Comment 40

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

:pike, Just to let you know that we missed a string for ringtone and use a workaround to fix it.

Comment 41

[Francesco Lodolo [:flod]]

Clearing NI, since Pike he's basically responsible for the way these files are named (bug 990537 comment 64) :D

Files were already discussed and approved in the original bugs, we just didn't realize the multi-locale build wasn't picking them up.

Comment 42

[Salvador de la Puente González [:salva]]

v2.0: d889984833025f208cfd3f3c2c37c87940a529dc

Comment 43

[Julien Wajsberg [:julienw]]

Salva, I still don't see the label for "ringtones", or the SIM id in the SMS app, in current nightly 2.0 builds :/

Comment 44

[Salvador de la Puente González [:salva]]

You're right. For some reason, the required localized strings are not being included in the JSON yet. I tried building the latest Gaia repo with MULTILOCALE and PRODUCTION=1 and it's working for me. It would be really nice if I can take a look at the command building the nightly zips.

Can you ask who is in charge of this?

Comment 45

[Pete Moore [:pmoore][:pete]]

julienw
11:52:02 trying to find out who can help on how we build the nightly images for flame
11:52:46 I think bhearsum|afk or aki  did some of this work ?

pmoore
11:54:34 hi julienw

julienw
11:55:01 hey pmoore !

julienw
11:55:12 we have an issue with multilocale builds

pmoore
11:55:29 ok

julienw
11:56:01 it's in bug 1043662

pmoore
11:56:22 thanks - let me take a look

julienw
11:56:24 we fixed the issue in Gaia build system but it looks like the fix is not used in the builds

pmoore
11:56:32 is it a config change?

julienw
11:56:58 no, it will likely be a code change
11:57:09 the first step would be to look at the commands used to build
11:57:18 maybe we'll find what's missing then

pmoore
11:57:24 sure
11:57:34 sorry, was just wrapping up something else - now you have my full attention

julienw
11:57:37 thanks a lot :)
11:57:41 simone is now known as simone|away

pmoore
11:57:47 so the first thing we can look at it is the log from the nightly on tbpl
11:58:16 this should show us the options that the script was called with
11:58:31 i'll see if i can find it
11:58:55 Bebe is now known as Bebe|food
11:58:55 gerard-majax_ left the room (quit: Quit: Ex-Chat).
11:59:21 gerard-majax_ [[e-mail]] entered the room.

julienw
12:00:07 pmoore, thanks
12:00:22 vebb left the room (quit: Quit: preprocessor-irc-cli).

julienw
12:00:25 (note, I need to go out for ~ 20 minutes, will get back soon)

pmoore
12:00:25 it looks like this was the nightly: http://buildbot-master85.srv.releng.scl3.mozilla.com:8001/builders/b2g_mozilla-central_flame_nightly/builds/26

julienw
12:00:45 thanks I'll look at this when I come back

pmoore
12:00:58 scripts/scripts/b2g_build.py --target flame --config b2g/releng-private-updates.py --gaia-languages-file locales/languages_all.json --gecko-languages-file gecko/b2g/locales/all-locales --config balrog/production.py
 in dir /builds/slave/b2g_m-cen_flame_ntly-000000000/. (timeout 3600 secs) (maxTime 14400 secs)
12:01:00 ok
12:01:02 speak later
12:01:09 http://buildbot-master85.srv.releng.scl3.mozilla.com:8001/builders/b2g_mozilla-central_flame_nightly/builds/26/steps/run_script/logs/stdio

Comment 46

[Pete Moore [:pmoore][:pete]]

The nightly was built from: http://hg.mozilla.org/mozilla-central/rev/37ac55a2601427374cf281b732852d94a9040111

It might be worth checking if the fix is included in this revision.

Comment 47

[Pete Moore [:pmoore][:pete]]

And scripts/scripts/b2g_build.py is: http://hg.mozilla.org/build/mozharness/file/tip/scripts/b2g_build.py

Comment 48

[Julien Wajsberg [:julienw]]

Thanks Pete !

I think Salva can reproduce the issue locally now, removing NI.

Comment 49

[Pete Moore [:pmoore][:pete]]

We chatted in IRC about this further, and now Salvador is able to reproduce the problem locally.

The nightly build of 2.0 (gaia git rev fb2dd31abed2803eb7ad67eb4c52abb48de1e0f7) included Salvador's fix:
https://github.com/mozilla-b2g/gaia/blob/fb2dd31abed2803eb7ad67eb4c52abb48de1e0f7/build/multilocale.js#L14

The logs is here:
https://tbpl.mozilla.org/php/getParsedLog.php?id=46152721&tree=Mozilla-B2g32-v2.0&full=1

The build script is here:
http://hg.mozilla.org/build/mozharness/file/tip/scripts/b2g_build.py

The way we traced the gaia commit id was to look at tbpl 2.0 tree to find latest nightly, take the hg revision, look at b2g/config/gaia.json to get the hg gaia changeset sha and url path, from there we could look up git commit SHA from mapper service, and from there we could browse the code on github.

i.e.
https://tbpl.mozilla.org/
-> https://tbpl.mozilla.org/?tree=Mozilla-B2g32-v2.0 (selected correct tree)
-> https://tbpl.mozilla.org/?tree=Mozilla-B2g32-v2.0&rev=09f7a7184c71 (latest push with Nightly build)
-> http://hg.mozilla.org/releases/mozilla-b2g32_v2_0/file/09f7a7184c71/b2g/config/gaia.json
-> http://hg.mozilla.org/integration/gaia-2_0/file/c29a28ac47b28ece9f7f154558348b0591643067
-> http://cruncher.build.mozilla.org/mapper/gaia/git/c29a28ac47b28ece9f7f154558348b0591643067
-> https://github.com/mozilla-b2g/gaia/commit/fb2dd31abed2803eb7ad67eb4c52abb48de1e0f7
-> https://github.com/mozilla-b2g/gaia/blob/fb2dd31abed2803eb7ad67eb4c52abb48de1e0f7/build/multilocale.js#L14

(in case there is a similar issue and somebody needs to trace the path, this is how to do it)

Comment 50

[Salvador de la Puente González [:salva]]

Sorry. The regular expression in the patch is incorrect. I don't know how I passed my tests but I would have swore it works. I'm going to revert the patch in order to upload the correct one.

v2.0 revert: 1a215917df01bb815811f33665bd3fdca4130708

Comment 51

[Salvador de la Puente González [:salva]]

Attachment: For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

As said, the regular expression was wrong. Quite weird but now it works.

Comment 52

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8474621 [details] [review]
For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

salva,

please add an assertion to verify result in build system integration test:

https://github.com/mozilla-b2g/gaia/blob/v2.0/build/test/integration/multilocale.test.js#L44-L49

you can use |make build-test-integration| or below command to to execute test:

> ./node_modules/.bin/mocha --harmony --reporter spec --ui tdd --timeout 0 build/test/integration/multilocale.test.js

and the assertion should look like:

> assert.isNotNull(zip.getEntry('locales/sounds-hacky-2.0.properties'), 'description here...');

Comment 53

[Salvador de la Puente González [:salva]]

That's sound great. Going on!

Comment 54

[Salvador de la Puente González [:salva]]

Comment on attachment 8474621 [details] [review]
For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

Now with tests. Could you review my patch, please?

Comment 55

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8474621 [details] [review]
For v2.0 only! Preventing *-hacky-2.0.properties from being deleted since then, their entries won't be included in JSON l10n dictionaries during the webapps-optimize build stage

that looks good! r=yurenju if build test is green on gaia try server.

Comment 56

[Salvador de la Puente González [:salva]]

I'm getting errors [1] but it does not seem very related with this bug.

What should I do?

[1] https://tbpl.mozilla.org/?rev=886b43748854c66efd65ce377a24fd96b677dfb8&tree=Gaia-Try

Comment 57

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

you still can land your commit since it is an intermittent failing test and bug was filed on bug 1043870.

Comment 58

[Salvador de la Puente González [:salva]]

v2.0: 6d14df4331b12f0cd89d473d2835a41ef5b7786d