Closed Bug 1186174 Opened 4 years ago Closed 4 years ago

Enable region specific search defaults for all Fennec users

Categories

(Firefox for Android :: General, defect, major)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox40 --- unaffected
firefox41 + fixed
firefox42 --- fixed

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
(Mark Finkle (:mfinkle) from bug 1175218 comment #34)
> Looks like this broke geo specific defaults on Fennec. The default for US
> should be "Yahoo" but is "Google".

This is because there are two mobile-l10n.js files, and for multi-locales builds, the one uses is the one in the installer/ folder which we didn't modify in bug 1175218. I think rather than enabling for all locales on multi-locale builds + en-US builds (which kinda covers all the possible cases) it makes more sense to just enable in mobile.js for all Fennec builds.
Attachment #8636772 - Flags: review?(mark.finkle)
Comment on attachment 8636772 [details] [diff] [review]
Patch

We'll need some testing to know that non-US clients are correctly getting Google.
Attachment #8636772 - Flags: review?(mark.finkle) → review+
Comment on attachment 8636772 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1175218
[User impact if declined]: Google set as the default on Fennec in the US.
[Describe test coverage new/current, TreeHerder]: none.
[Risks and why]: low, the patch just changes prefs to enable the feature for all locales.
[String/UUID change made/needed]: none.
Attachment #8636772 - Flags: approval-mozilla-aurora?
This was tested quickly by sebastian on #mobile, worked as expected.
https://hg.mozilla.org/mozilla-central/rev/9f71fd7e13f9
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Florian - I assume this will need to be uplifted to Beta as well in order to take bug 1175218 on Beta. Can you please confirm?
Flags: needinfo?(florian)
Tracking 40 and 41 as we can't ship with this bug.
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #6)
> Florian - I assume this will need to be uplifted to Beta as well in order to
> take bug 1175218 on Beta. Can you please confirm?

Yes. I was assuming I would just fold this patch into the beta patch from bug 1175218, but if you prefer us to uplift this as a separate changeset on the beta repository, that's fine with me too.
Flags: needinfo?(florian)
(In reply to Florian Quèze [:florian] [:flo] (PTO until August 3rd) from comment #8)
> Yes. I was assuming I would just fold this patch into the beta patch from
> bug 1175218, but if you prefer us to uplift this as a separate changeset on
> the beta repository, that's fine with me too.

I don't have a preference. Either way is fine for me.
Florian is going to roll this fix into his patch on bug 1175218 for 40. Clearing tracking and marking 40 as unaffected.
Comment on attachment 8636772 [details] [diff] [review]
Patch

Given that this fix has already been tested, let's get this on Aurora asap. Aurora+
Attachment #8636772 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.