Closed Opened 6 years ago Closed 6 years ago

# make Yahoo default for builds in North American timezones

Not set
normal

## Tracking

### ()

VERIFIED FIXED
Firefox 37
Tracking Status
firefox34 + verified
firefox35 + verified
firefox36 + verified
firefox37 --- verified
firefox-esr31 35+ affected

## Attachments

### (2 files, 2 obsolete files)

 9.22 KB, patch Dolske : review+ Details | Diff | Splinter Review 3.23 KB, patch Details | Diff | Splinter Review
- need new URLs for the yahoo description
- need to adjust default search prefs in region.properties to make Yahoo default
Depends on: 1102297
Depends on: 1102295
Attached patch patch for 34.1 (obsolete) — Splinter Review
This isn't using bug 1102295 yet, but it could (for Mac).

I think this is fairly self-documented - I'm adding special ".US" variants of the default engine pref, and having the code dynamically use them instead of the non-US prefs when the system clock timezone falls within the range of "North America" (or South America, but we decided that was OK). Far from perfect, but better than nothing.

Note that this also means that the default can change at every Firefox startup - I could set a profile pref to avoid that (i.e. make getIsUS persist its decision after the first time), but that seemed needlessly complicated.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #8526850 - Flags: review?(dolske)
Attachment #8526850 - Flags: review?(benjamin)
Comment on attachment 8526850 [details] [diff] [review]
patch for 34.1

I like the simplicity.
Attachment #8526850 - Flags: review?(benjamin) → review+
Comment on attachment 8526850 [details] [diff] [review]
patch for 34.1

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

::: toolkit/components/search/nsSearchService.js
@@ +4177,5 @@
> +      if (getIsUS()) {
> +        defPref += "defaultenginename.US";
> +      } else {
> +        defPref += "defaultenginename";
> +      }

Hmm. You're changing the |defaultEngine| getter here, but not the setter a dozen lines down. I haven't looked at what uses that code, but it would imply someone calling |.defaultEngine = "foo"| would have no effect in the US, because we're writing and reading different prefs.

Seems like the setter should also adjust what pref it sets. Or we could do something more complicated (ugh) by always using the the non-US pref when there's a userPref, and otherwise falling back to the geospecific pref when for the factory-set default. But that sounds like PITA.
Comment on attachment 8526850 [details] [diff] [review]
patch for 34.1

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

::: toolkit/components/search/nsSearchService.js
@@ +402,5 @@
>  }
>
> +// Hacky methods that tries to determine if this user is in a US geography, and
> +// using an en-US build.
> +function getIsUS() {

Hmm, I think it would be a good idea to make this a 1-time lookup. I.E. if browser.search.isUS is not set (no default is shipped), then do the timezone lookup to set it true/false and use that value in the future.

If someone travels from their home India to the US, they shouldn't be surprised by their Firefox search engine changing.
Summary: Yahoo search engine changes → make Yahoo default for builds in North American timezones
(In reply to Justin Dolske [:Dolske] from comment #3)
> Hmm. You're changing the |defaultEngine| getter here, but not the setter a
> dozen lines down. I haven't looked at what uses that code, but it would
> imply someone calling |.defaultEngine = "foo"| would have no effect in the
> US, because we're writing and reading different prefs.'

I don't actually need to change this getter, given how Firefox uses the search service - we actually only rely on the _originalDefaultEngine fallback in the selectedEngine getter. So I'm only going to change _originalDefaultEngine's getter, and avoid touching the defaultEngine getter/setter (which Firefox ensures stays consistent with .currentEngine via the code at http://hg.mozilla.org/releases/mozilla-beta/annotate/default/browser/components/nsBrowserGlue.js#l375.

(In reply to Justin Dolske [:Dolske] from comment #4)
> Hmm, I think it would be a good idea to make this a 1-time lookup. I.E. if
> browser.search.isUS is not set (no default is shipped), then do the timezone
> lookup to set it true/false and use that value in the future.

I did this. This also has the benefit of being able to override all of this magical behavior via hotfix, should it be needed.
Attached patch patch for 34.0.5 (obsolete) — Splinter Review
(I also updated all of the order prefs, I was missing one in the previous patch.)
Attachment #8527445 - Flags: review?(dolske)
Attachment #8526850 - Attachment is obsolete: true
Attachment #8526850 - Flags: review?(dolske)
Had to adjust a couple of tests to account for the change in default.
Attachment #8527445 - Attachment is obsolete: true
Attachment #8527445 - Flags: review?(dolske)
Attachment #8527745 - Flags: review?(dolske)
Summary: make Yahoo default for builds in North American timezones → [34.0.5 only] make Yahoo default for builds in North American timezones
Attachment #8527745 - Flags: review?(dolske) → review+
Flags: qe-verify+
Pushed to release, for 34.0.5 only:
https://hg.mozilla.org/releases/mozilla-release/rev/2af28d4d94db
This is causing xpcshell permafail. Please fix or backout ASAP (per tree rules, you should have been watching this push to begin with).
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=25577&repo=mozilla-release
Flags: needinfo?(gavin.sharp)
Pushed test fixes:
https://hg.mozilla.org/releases/mozilla-release/rev/f2f39afa775b
Flags: needinfo?(gavin.sharp)
And another:
https://hg.mozilla.org/releases/mozilla-release/rev/62287d20bf35
Verified that Yahoo! is set as default after fresh install with a clean profile for en-US builds - US timezone.

With an older profile where another search engine was set as default - that one remains default on 34.0.5 and there is no pop-up (UI tour) about Yahoo. I suppose this will be active only after release.

Build: 34.0.5
Environments: Windows 7 64-bit, Windows 8 32-bit, Mac OS X 10.9.5, Ubuntu 12.04 LTS 32-bit
(In reply to Petruta Rasa [QA] [:petruta] from comment #12)
> With an older profile where another search engine was set as default - that
> one remains default on 34.0.5 and there is no pop-up (UI tour) about Yahoo.
> I suppose this will be active only after release.

If you had a non-default engine selected (i.e. not Google), then this change should have no impact. If you had Google selected, this change should cause Yahoo to be selected post-update (if you are in a US timezone).
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #13)
> (In reply to Petruta Rasa [QA] [:petruta] from comment #12)
> > With an older profile where another search engine was set as default - that
> > one remains default on 34.0.5 and there is no pop-up (UI tour) about Yahoo.
> > I suppose this will be active only after release.
>
> If you had a non-default engine selected (i.e. not Google), then this change
> should have no impact. If you had Google selected, this change should cause
> Yahoo to be selected post-update (if you are in a US timezone).

So any user in the US timezone or using en_US builds? with have there default engine if there using the Google default automatically changed to yahoo.

Won't this cause allot of upset and frustration to users can't you leave it to new installs or new profiles only ?
(In reply to Toady from comment #14)
> Won't this cause allot of upset and frustration

Why frustration, if you can change engine _at any time_ with a couple clicks? This is not a lock-in strategy like many other companies are doing (and still, their users are not upset...)
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #13)
> If you had a non-default engine selected (i.e. not Google), then this change should have no impact. If you had Google selected, this change should cause Yahoo to be selected post-update (if you are in a US timezone).

I manually modified the timezone and set it to US.
If no modification were made to the search engine (Google remained as default), after update Yahoo is set as default (no notifications).

If the search engine was manually selected and a pre-installed search engine was chosen (eg: Bing, DuckDuckGo etc), after update the selected search engine will remain as main search engine.
This also happens for manually added search engines (eg Google Translate, Youtube etc).
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b232181e8006

https://hg.mozilla.org/mozilla-central/rev/17de0f463944
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Summary: [34.0.5 only] make Yahoo default for builds in North American timezones → make Yahoo default for builds in North American timezones
Target Milestone: --- → Firefox 37
Trying to land this on aurora pointed out that my fix wasn't exactly l10n-friendly. Rather than putting the hacky US-only prefs in region.properties, this just hardcodes them in firefox.js.
Landed that followup, will include it in the Aurora/Beta patches:
https://hg.mozilla.org/mozilla-central/rev/d103d1908a34
https://hg.mozilla.org/releases/mozilla-beta/rev/70dca05c9575
https://hg.mozilla.org/releases/mozilla-aurora/rev/94333fbcde0b
Had to back this out of beta because a test was failing:
https://hg.mozilla.org/releases/mozilla-beta/rev/28157652a0fd
Depends on: 1106150
Relanded together with bug Bug 1101122 which removes the test that was failing (because it no longer applies to the new interface)
https://hg.mozilla.org/releases/mozilla-beta/rev/c77db3659462
Is is just me or does

*|* {
-moz-user-select: text;
}

in toolkit\themes\shared\in-content\common.inc.css break the checkboxs in the in-content preferences one click search engine list richlistbox.

in 34.0+
Depends on: 1106349
Checked that Yahoo is the default search engine under Win 7 64-bit, Ubuntu 14.04 64-bit and Mac OSX 10.9.5 in US timezone starting Firefox with a clean profile.
Builds used Firefox 35 beta 1, Developer Edition 36.0a2 and Nightly 37.0a1
Status: RESOLVED → VERIFIED
Depends on: 1108627
Depends on: 1109118
Depends on: 1109120
No longer depends on: 1102295
No longer depends on: 1102297
Depends on: 1117186
You need to log in before you can comment on or make changes to this bug.