Closed Bug 1267320 Opened 8 years ago Closed 8 years ago

Multiple spellings for telemetry extras

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: liuche, Assigned: mfinkle, Mentored)

Details

(Whiteboard: [lang=java][good next bug])

Attachments

(2 files, 1 obsolete file)

We're getting multiple spellings for telemetry extras, likely from string conversions from Enums in other locales.

There should be a way to convert enums to only the en-US locale because we're using this for usage analysis and don't need locale separations.

For example:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/widget/LoginDoorHanger.java#86
So I guess we need to decide:
* Use the strings "as-is" which probably makes an ALLCAPS string
* Use .toLowerCase(Locale.US) to create an English-friendly lowercase (the enums are all in English anyway)
* Override the Enum's toString method and do the lowercase there, instead of in all uses.
Hello, I am a new contributor and i am willing to work on this bug. Please, Can you guide me to go forward with it.
Flags: needinfo?(liuche)
Hi, I would like to contribute in solving this bug. I am new to this, can you please guide me in how to proceed ahead?
This doesn't sound like a good first bug, since it requires some investigation.
Whiteboard: [lang=java][good first bug] → [lang=java][good next bug]
Here's a patch that uses the .toLowerCase(Locale.ENGLISH) approach
Assignee: nobody → mark.finkle
Attachment #8747758 - Flags: review?(liuche)
Testing the patch, I see:

Before patch, in en-US:
D/Telemetry(32229): SendUIEvent: event = show.1 method = doorhanger timestamp = 8112936571 extras = login

Before patch, in tr_TR:
D/Telemetry( 1188): SendUIEvent: event = show.1 method = doorhanger timestamp = 8113023102 extras = logın

After patch, in tr_TR:
D/Telemetry( 2059): SendUIEvent: event = show.1 method = doorhanger timestamp = 8113106198 extras = login

The 'i' in "login" looks as expected after the patch.
Comment on attachment 8747758 [details] [diff] [review]
doorhanger-telemetry-string-fix v0.1

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

http://developer.android.com/reference/java/util/Locale.html#default_locale

Android docs suggest using Locale.US, because it will be present on all devices. r+ with that change.
Attachment #8747758 - Flags: review?(liuche) → review+
Flags: needinfo?(liuche)
Locale.ENGLISH -> Locale.US
Attachment #8747758 - Attachment is obsolete: true
Attachment #8750022 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c1cb391d03b8
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8750022 [details] [diff] [review]
doorhanger-telemetry-string-fix v0.2

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Multiple spellings for a single telemetry probe
[Describe test coverage new/current, TreeHerder]: Running on Nightly
[Risks and why]: Low risk. The change is limited to converting a telemetry probe text input to lowercase using US locale settings so the lower casing is done consistently across other locales.
[String/UUID change made/needed]: none
Attachment #8750022 - Flags: approval-mozilla-aurora?
Comment on attachment 8750022 [details] [diff] [review]
doorhanger-telemetry-string-fix v0.2

Corrects lowercasing on telemetry, let's uplift.
Attachment #8750022 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This would be good to verify. Chenxia can you confirm the fix worked either in nightly or once it lands on aurora?
Flags: needinfo?(liuche)
Flags: needinfo?(liuche)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.