786856 - Only prompt for feedback in locales where about:feedback is localized

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Attachment: patch

Localizers may not have time to localize the about:feedback strings if we decide to uplift it to beta (it's currently in Aurora), so we should only show it to users if it's available in their locale.

I modeled this patch after the way we keep our url shortener redirect white list. I'm not sure how we'd want to go about landing it, though, since it will only really be needed for Firefox 16 after we uplift the feedback feature to beta.

(Also note, users would still be able to navigate to about:feedback, but it wouldn't necessarily be localized.)

Comment 1

[Axel Hecht [:Pike]]

Can you patch mobile/android/locales/filter.py in addition so that we don't report the strings if they land on beta?

Also probably good to add comments to the code that this is beta 16 only code.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 656649 [details] [diff] [review]
patch

This looks OK. Make sure you test with a few different locales on a device.

Note for those watching: We plan to remove this code when l10n has sufficient time to get the strings translated.

I think it's OK to land this on Nightly, Aurora and Beta. Any l10n groups needing to test can use about:feedback directly.

Comment 3

[Axel Hecht [:Pike]]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 656649 [details] [diff] [review]
> patch
> I think it's OK to land this on Nightly, Aurora and Beta. Any l10n groups
> needing to test can use about:feedback directly.

I disagree, I'd expect feedback to come from other aurora and nightly channels, too. Also, it's completely unclear when we'd remove this code, if we add it across all channels now.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Axel Hecht [:Pike] from comment #3)
> (In reply to Mark Finkle (:mfinkle) from comment #2)
> > Comment on attachment 656649 [details] [diff] [review]
> > patch
> > I think it's OK to land this on Nightly, Aurora and Beta. Any l10n groups
> > needing to test can use about:feedback directly.
> 
> I disagree, I'd expect feedback to come from other aurora and nightly
> channels, too. Also, it's completely unclear when we'd remove this code, if
> we add it across all channels now.

I won't push back too hard on landing the en_US filter on beta only, but we do have a feature in there that fails to work on any release that is not in the google play store. We send people to a 404 for release notes in nightly and aurora too, so maybe sending them to a 404 for the google play store is fine?

Comment 5

[:Margaret Leibovic]

Comment on attachment 656649 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): feedback solicitation (bug 774479)
User impact if declined: some locales may not have translated strings for feedback page
Testing completed (on m-c, etc.): n/a, this only affects Firefox 16 when we uplift bug 774479 to beta, localizers will have the time to translate the strings by the time Firefox 17 ships
Risk to taking this patch (and alternatives if risky): low-risk locale check, but we'll want to verify changes on beta, since we're don't want to land this on central/aurora
String or UUID changes made by this patch: n/a

Comment 6

[:Margaret Leibovic]

We only want this for Firefox 16, so marking 17 and 18 as unaffected.

https://hg.mozilla.org/releases/mozilla-beta/rev/5eac8f94b51b

Comment 7

[Aaron Train [:aaronmt]]

Using mozilla-beta (16), beta-candidate #2 I tested the following on my Galaxy Nexus (Android 4.1.1) and all passed.

* Czech (cs)
* Danish (da)	
* Finnish (fi)		
* Korean (ko)
* Italian (it)		
* Norwegian bokmal (nb-NO)		
* Dutch (nl)		
* Brazilian Portuguese (pt-BR)		
* Portuguese (pt-PT)		
* Russian(ru)		
* German (de)		
* Spanish (es-ES)		
* French (fr)		
* Japanese (ja)		
* Polish (pl)

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Aaron Train [:aaronmt] from comment #7)
> Using mozilla-beta (16), beta-candidate #2 I tested the following on my
> Galaxy Nexus (Android 4.1.1) and all passed.

passed == did not display because they are are en-US?

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #8)
> (In reply to Aaron Train [:aaronmt] from comment #7)
> > Using mozilla-beta (16), beta-candidate #2 I tested the following on my
> > Galaxy Nexus (Android 4.1.1) and all passed.
> 
> passed == did not display because they are are en-US?

... are not en-US

Comment 10

[Aaron Train [:aaronmt]]

(In reply to Mark Finkle (:mfinkle) from comment #9)
> ... are not en-US
Yes. They did not open about:feedback after the conditions from bug 774479 comment 38.