Shing Lyu [:shinglyu] Reporter
	Description • 9 years ago

*** Description
the "locale" data is not correct for the FTU ping

*** Steps to Reproduce
1. Flash the phone
2. Select 繁體中文 (zh-TW) in FTU's language selection
3. Finish the FTU, connect to Wi-Fi during the FTU
4. Wait for the FTU ping to reach the metrics server

*** Expected Results
The "locale" field in FTU should be "zh-TW"

*** Actual Results
The "locale" field in FTU is "en-US"

*** Other Notes
Even if I postpone the FTU ping by 1) connect to wifi after FTU 2) Insert SIM after FTU, the FTU ping still can't get the correct locale

*** Reproduction Frequency
100%

*** Build
Note: this happens both on Flame and Nexus5

  Gaia-Rev        014d38f7ad3912b8b33cb08ce7535a5dc5aced59
  Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7ba1778d237b
  Build-ID        20150323162503
  Version         37.0
  Device-Name     hammerhead
  FW-Release      5.0
  FW-Incremental  eng.cltbld.20150323.202104
  FW-Date         Mon Mar 23 20:21:24 EDT 2015
  Bootloader      HHZ12d

	Marshall Culpepper [:marshall_law]
	Comment 1 • 9 years ago

It looks like we initialize the locale as soon as the FTU ping begins, and don't update it. The fix would be pretty simple, just populate the locale before we submit the ping, instead of at initialization time.

Do we normally ship with en-US as the default locale in our production builds?

	Shing Lyu [:shinglyu] Reporter
	Comment 2 • 9 years ago

That's what I suspected. 

I guess it's more of a business decision than a technical one. Should I NI Ravi or anyone to make the call?

	Marshall Culpepper [:marshall_law]
	Comment 3 • 9 years ago

Yeah, Ravi would be best.. sorry, I'll NI him again. Just for clarification Ravi, we will get the locale of the device/carrier specific build, but we won't be able to tell if the user changes the locale before the ping is submitted.

	Ravi Dandu
	Comment 4 • 9 years ago

Users may change locale based on their language. Fix looks simple. Lets fix it for 2.2.

	Hema Koka [:hema]
	Comment 5 • 9 years ago

Tamara/Marshall: Who can take this?

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 6 • 9 years ago

I can put a patch together for this. Looks like we just need to move the `self._pingData.locale = window.navigator.language;` to the actual ping method, in the same way that we populate the time.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 8 • 9 years ago

Comment on attachment 8589835 [details] [review]
[gaia] sfoster:ftu-ping-locale-bug-1146789 > mozilla-b2g:master

How does this look? I didn't add a new test for the late change to locale case, but can do if we think it adds value.

	(inactive after 6/18) Alive Kuo [:alive]
	Comment 9 • 9 years ago

Comment on attachment 8589835 [details] [review]
[gaia] sfoster:ftu-ping-locale-bug-1146789 > mozilla-b2g:master

I have poor knowledge about this, but if Marshall who creates this module thinks it
's fine you have my sign on. (What about test?)

	Gregor Wagner [:gwagner]
	Comment 10 • 9 years ago

Lets uplift once done.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 11 • 9 years ago

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #9)
 
> I have poor knowledge about this, but if Marshall who creates this module
> thinks it's fine you have my sign on. (What about test?)

Yeah I'll work on a test for this specific issue.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 12 • 9 years ago

PR updated with unit test coverage for ensuring we actually call getPingData, and that a late change in navigator.language gets picked up.

	Marshall Culpepper [:marshall_law]
	Comment 13 • 9 years ago

Comment on attachment 8589835 [details] [review]
[gaia] sfoster:ftu-ping-locale-bug-1146789 > mozilla-b2g:master

Looks good. I left a comment about naming for the getter function in the pull request

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 14 • 9 years ago

I renamed getPingData to assemblePingData. Tests are updated and results on treeherder are all green.

	GitHub Autolander Bot
	Comment 15 • 9 years ago

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/e768af6558957ddb0f6a9ce579ea41c3e3d0b203

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 16 • 9 years ago

Marshall, do you want to make the case to get this uplifted to 2.2? I'd assume that having accurate locale data is important, but in the systemsfe triage we decided not to block on this.