Closed Bug 1024757 Opened 7 years ago Closed 7 years ago

Translation opportunity FHR should include languages not in the supported list


(Firefox :: Translation, defect)

Not set



Firefox 33
Tracking Status
firefox32 --- verified
firefox33 --- verified


(Reporter: Felipe, Assigned: Felipe)



(Whiteboard: p=1 s=33.1 [qa!])


(1 file, 1 obsolete file)

The proper definition of the translation opportunity metric that we want to measure is any language not in the user's target language, independent of us showing the infobar (because that language is supported) or not.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → felipc
Attachment #8439650 - Flags: review?(mdeboer)
Flags: firefox-backlog+
Whiteboard: p=1 [qa+]
Hi Felipe, I didn't receive a notification if you would be taking Bug 1024757 this iteration?
Flags: needinfo?(felipc)
Marco, can you include this in the current iteration?

Flags: needinfo?(felipc) → needinfo?(mmucci)
Comment on attachment 8439650 [details] [diff] [review]

Review of attachment 8439650 [details] [diff] [review]:

r=me _if_ the question I asked below can be answered with 'nay'.

Thanks for this one!

::: browser/components/translation/Translation.jsm
@@ +49,2 @@
>          return;
> +      }

nit: no need for braces for this single line block.

@@ +53,3 @@
>        TranslationHealthReport.recordTranslationOpportunity(aData.detectedLanguage);
> +
> +      if (this.supportedSourceLanguages.indexOf(aData.detectedLanguage) == -1) {

Question: do want to measure this separately, or can this information be filtered easily when analyzing the data?

It's just that an additional datapoint to filter the translation-opportunities with seems easier to me than to keep a list around and filter the numbers manually.

@@ +53,5 @@
>        TranslationHealthReport.recordTranslationOpportunity(aData.detectedLanguage);
> +
> +      if (this.supportedSourceLanguages.indexOf(aData.detectedLanguage) == -1) {
> +        return;
> +      }

nit: same.
Attachment #8439650 - Flags: review?(mdeboer) → review+
Oh, and don't forget a nice commit message! ;)
Added to Iteration 33.1
Flags: needinfo?(mmucci)
Whiteboard: p=1 [qa+] → p=1 s=33.1 [qa+]
I checked your suggestion with the Metrics people and they preferred a separate list for unsupported languages as you suggested.
Attachment #8439650 - Attachment is obsolete: true
Attachment #8442857 - Flags: review?(mdeboer)
Comment on attachment 8442857 [details] [diff] [review]
Separate list of unsupported languages

Review of attachment 8442857 [details] [diff] [review]:

Yikes, looks like my suggestion blew up this patch! But, it's for the best... sorry for the inconvenience ;)


::: browser/components/translation/Translation.jsm
@@ +257,5 @@
> +   *        The language of the page.
> +   */
> +  recordMissedTranslationOpportunity: function (language) {
> +    this._withProvider(provider => provider.recordMissedTranslationOpportunity(language));
> +   },

nit: indentation looks wrong here.
Attachment #8442857 - Flags: review?(mdeboer) → review+
Np, there was a lot of copy&pasting from the other measurement :)
Comment on attachment 8442857 [details] [diff] [review]
Separate list of unsupported languages

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug is part of the automatic translation feature, which we want to A/B with a subset of Aurora 32 users.
User impact if declined: Page languages that are unsupported by the translation service won't be reported through FHR
Testing completed (on m-c, etc.): landed on inbound
Risk to taking this patch (and alternatives if risky): small
String or IDL/UUID changes made by this patch: none
Attachment #8442857 - Flags: approval-mozilla-aurora?
Flags: in-testsuite+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Hi Florin, can a QA contact be assigned for verification of this bug.
Flags: needinfo?(florin.mezei)
How to verify this bug: 

- make sure telemetry is enabled
- enable language detection by enabling these two prefs:

Visit a page in a language not on the supported list. e.g., italian, dutch, finnish..

Verify that the FHR payload contains measurements about these languages under the "missed opportunities" counter and list.
Flags: needinfo?(florin.mezei)
QA Contact: bogdan.maris
Comment on attachment 8442857 [details] [diff] [review]
Separate list of unsupported languages

Aurora approval granted.
Attachment #8442857 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I was able to confirm that the "missedTranslationOpportunityCount" and "missedTranslationOpportunityCountsByLanguage" metrics are working properly on:
 * Nightly 33 (2014-06-22),
 * Aurora 32 (2014-06-22),
using Windows 7 64-bit, Windows 8.1 64-bit, Mac OS X 10.9.2 and Ubuntu 14.04 LTS 32-bit.
Whiteboard: p=1 s=33.1 [qa+] → p=1 s=33.1 [qa!]
I pushed the follow-up comment to aurora to keep the documentation fully correct there. This is just a documentation file, NPOTB
You need to log in before you can comment on or make changes to this bug.