Closed Bug 786856 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android :: General, defect)

16 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox16 --- verified
firefox17 --- unaffected
firefox18 --- unaffected

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

(Whiteboard: [playreview])

Attachments

(1 file)

Attached patch patchSplinter Review
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.)
Attachment #656649 - Flags: review?(mark.finkle)
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 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.
Attachment #656649 - Flags: review?(mark.finkle) → review+
(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.
(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 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
Attachment #656649 - Flags: approval-mozilla-beta?
Version: Trunk → Firefox 16
Depends on: 787150
Whiteboard: [playreview]
Attachment #656649 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We only want this for Firefox 16, so marking 17 and 18 as unaffected.

https://hg.mozilla.org/releases/mozilla-beta/rev/5eac8f94b51b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Blocks: 788239
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)
Status: RESOLVED → VERIFIED
(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?
(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
(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.
You need to log in before you can comment on or make changes to this bug.