1102416 - make Yahoo default for builds in North American timezones

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

- need new URLs for the yahoo description
- need to adjust default search prefs in region.properties to make Yahoo default

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch for 34.1

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.

Comment 2

[Benjamin Smedberg]

Comment on attachment 8526850 [details] [diff] [review]
patch for 34.1

I like the simplicity.

Comment 3

[Justin Dolske [:Dolske]]

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 4

[Justin Dolske [:Dolske]]

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.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(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.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch for 34.0.5

(I also updated all of the order prefs, I was missing one in the previous patch.)

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch for 34.0.5 including test fixes

Had to adjust a couple of tests to account for the change in default.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Pushed to release, for 34.0.5 only:
https://hg.mozilla.org/releases/mozilla-release/rev/2af28d4d94db

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Pushed test fixes:
https://hg.mozilla.org/releases/mozilla-release/rev/f2f39afa775b

Comment 11

[:Gavin Sharp [email: [e-mail]]]

And another:
https://hg.mozilla.org/releases/mozilla-release/rev/62287d20bf35

Comment 12

[Petruta Horea [:phorea], Desktop Test Engineering]

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

Comment 13

[:Gavin Sharp [email: [e-mail]]]

(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).

Comment 14

[Toady]

(In reply to :Gavin Sharp [email: [e-mail]] 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).

Am i reading this correctly

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 ?

Comment 15

[Marco Bonardo [:mak]]

(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...)

Comment 16

[Petruta Horea [:phorea], Desktop Test Engineering]

(In reply to :Gavin Sharp [email: [e-mail]] 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).

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b232181e8006

https://hg.mozilla.org/mozilla-central/rev/17de0f463944

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Attachment: followup l10n fix

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.

Comment 19

[:Gavin Sharp [email: [e-mail]]]

Landed that followup, will include it in the Aurora/Beta patches:
https://hg.mozilla.org/mozilla-central/rev/d103d1908a34

Comment 20

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-beta/rev/70dca05c9575
https://hg.mozilla.org/releases/mozilla-aurora/rev/94333fbcde0b

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Had to back this out of beta because a test was failing:
https://hg.mozilla.org/releases/mozilla-beta/rev/28157652a0fd

Comment 22

[:Felipe Gomes (needinfo for replies!)]

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

Comment 23

[Toady]

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+

Comment 24

[Petruta Horea [:phorea], Desktop Test Engineering]

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