Closed Bug 1061736 Opened 5 years ago Closed 5 years ago

Add DuckDuckGo to default search engine list

Categories

(Firefox :: Search, defect)

33 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 36
Tracking Status
firefox33 + verified
firefox34 + verified
firefox35 + verified
firefox36 + verified
relnote-firefox --- 33+

People

(Reporter: mconnor, Assigned: Gavin)

References

(Depends on 1 open bug)

Details

(Keywords: feature)

Attachments

(1 file, 4 obsolete files)

The current target date is Nov 9th to launch, in some form.  We're meeting with PMM/Comms to sort out the exact plan.  This is planned to ship on both Desktop and Android, I'll pull together patches for both.
What have they said is their supported locales (and do we agree with that list : ) )?
OS: Mac OS X → All
Hardware: x86 → All
We have the right, but no obligation (in any specific locale) to include them in the search drop-down.  Our initial plan is to include them in en-US, and to update the strings for any locale that already included them.  Here's a link to their list of regions:  https://duckduckgo.com/params
Keywords: feature
Summary: Add DuckDuckGo to default search engine list. → Add DuckDuckGo to default search engine list
Release Note Request (optional, but appreciated)
[Why is this notable]: We're going to want to relnote this search provider addition.
[Suggested wording]: Added DuckDuckGo search provider
[Links (documentation, blog post, etc)]: I trust there will be a blog post about to which we can link.
Does this need to specifically ship in 33.0.1? Can we ship this in 33.0, in which case the change should land this week?
Flags: needinfo?(mconnor)
Flags: needinfo?(elancaster)
We want to announce this on the 9th, so we can't put it in 33. This is one of the main desktop drivers for 33.0.1 from my perspective.
(In reply to Lawrence Mandel [:lmandel] from comment #3)
> Release Note Request (optional, but appreciated)
> [Why is this notable]: We're going to want to relnote this search provider
> addition.
> [Suggested wording]: Added DuckDuckGo search provider
> [Links (documentation, blog post, etc)]: I trust there will be a blog post
> about to which we can link.

Since our role out is very similar I'd suggest using similar wording to the Twitter rel notes [1].

[Suggested wording]: Added DuckDuckGo to the search bar for the en-US locale. Additional locale support will be added in the future

[1] What's new section : http://website-archive.mozilla.org/www.mozilla.org/firefox_releasenotes/en-US/firefox/8.0.1/releasenotes/
Blocks: fx10
mconnor: does our agreement cover position/ordering?

We were planning to shoot to include this in en-US, de, ru, fr, br, pl, es, jp builds, though the exact list will be dependent on what translations we get for the privacy button (bug 1069300).

It looks like DDG handles Accept-Lang properly and does the right thing here, so I'm assuming that won't be an issue.
Flags: needinfo?(elancaster)
Zero to do with position or ordering.  I'd probably argue for fourth to keep general engines at the top. 

Fennec has a different list, ideally I'd like to coordinate those locales. One interesting data point would be to look at where DDG has traction with our users via FHR. It's a locale independent plugin, so the only issue is updating list.txt where we want it picked up.

I don't get the dependency on the privacy button translation, is the idea that we'll only produce 33.0.1 builds for those locales?
(In reply to Mike Connor [:mconnor] from comment #8)
> Zero to do with position or ordering.  I'd probably argue for fourth to keep
> general engines at the top. 

That would need a proper fix riding the train, given the amount of variants there are (mobile looks probably a bit better).

Take for example search.order for desktop:
de: 1. Google, 2. Yahoo
ja: complete mayhem
pl: 1. Google, doesn't ship Yahoo or Bing
ru: 1. Google, 2. Yandex
Note - the locale list suggested for mobile was based on FHR search data as well as in-market data around DDG uptake. 

See https://bugzilla.mozilla.org/show_bug.cgi?id=883254#c4

Although it is unlikely we'll now expand that list of locales within the add-on, but simply through promoting  / featuring the DDG search plug-in (add-on)
In chatting with Pike, we talked about a couple of options that would work for ordering, but it turns out the order prefs don't impact existing users anyways (bug 1073603). So let's not worry about ordering for 33.0.1, we'll fix it in 34 or 35.
(In reply to Mike Connor [:mconnor] from comment #8)
> I don't get the dependency on the privacy button translation, is the idea
> that we'll only produce 33.0.1 builds for those locales?

No, but the Privacy tour will be highlighting these features together. It's slightly better to bundle these features and have them hit individual locales at the same time in a later release (34/35) and then just run the tour for those locales then.
(In reply to Karen Rudnitski [:kar] from comment #10)
> Note - the locale list suggested for mobile was based on FHR search data as
> well as in-market data around DDG uptake. 
> 
> See https://bugzilla.mozilla.org/show_bug.cgi?id=883254#c4

Given that list is market rather than locale based, I think there's a lot of overlap - Australia/Canada/UK/New Zealand mostly use en-US builds, France/Germany/Spain are on our list, I imagine Austrian users use mostly en-US or de builds? We're not targeting Mexico specifically due to l10n for that locale being under-staffed.

> Although it is unlikely we'll now expand that list of locales within the
> add-on, but simply through promoting  / featuring the DDG search plug-in
> (add-on)

Why don't you guys just ship DDG in 33.0.1 too?
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #13)

> > Although it is unlikely we'll now expand that list of locales within the
> > add-on, but simply through promoting  / featuring the DDG search plug-in
> > (add-on)
> 
> Why don't you guys just ship DDG in 33.0.1 too?

We are:
https://bugzilla.mozilla.org/show_bug.cgi?id=883254#c5
(In reply to Mark Finkle (:mfinkle) from comment #14)

> > Why don't you guys just ship DDG in 33.0.1 too?
> 
> We are:
> https://bugzilla.mozilla.org/show_bug.cgi?id=883254#c5

We are not shipping DDG in an add-on, but Karen's comment does not explcictly state that we are using Fx33.0.1, but I think we'll need to, if we want DDG in Fennec in November.
Depends on: 1073212
(In reply to Francesco Lodolo [:flod] from comment #9)
> (In reply to Mike Connor [:mconnor] from comment #8)
> > Zero to do with position or ordering.  I'd probably argue for fourth to keep
> > general engines at the top. 
> 
> That would need a proper fix riding the train, given the amount of variants
> there are (mobile looks probably a bit better).
> 
> Take for example search.order for desktop:
> de: 1. Google, 2. Yahoo
> ja: complete mayhem
> pl: 1. Google, doesn't ship Yahoo or Bing
> ru: 1. Google, 2. Yandex

We'd have to set the "right" value in the region.properties file.  The only real annoyance is the multiple repos, otherwise this is pretty straightforward.  That'd be third for de, second for pl, etc.  That said, I think we need to be more consistent, so a riding-the-trains fix would be good once the insanity has died down.
Flags: needinfo?(mconnor)
(In reply to Mike Connor [:mconnor] from comment #8)
> Zero to do with position or ordering.  I'd probably argue for fourth to keep
> general engines at the top. 

I think since you're taking the 4th position this doesn't matter.  But with Desktop there are contractual order requirements for both Google (#1) and Yahoo (#2).
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #17)
> (In reply to Mike Connor [:mconnor] from comment #8)
> > Zero to do with position or ordering.  I'd probably argue for fourth to keep
> > general engines at the top. 
> 
> I think since you're taking the 4th position this doesn't matter.  But with
> Desktop there are contractual order requirements for both Google (#1) and
> Yahoo (#2).

Quite true, for clarity I mean there are no requirements for DDG at all.

As a followup, the top 15 locales (in order) for current DDG adoption to date (based on FHR data):

en-US, de, fr, en-GB, es-ES, nl, pt-BR, it, es-MX, es-AR, sv-SE, pl, es-CL, ru, ja

The list is pretty similar as it stands to the list Gavin had, so we're pretty close.
As a followup after discussion with Gavin: I'd like to make sure we still add en-GB, based solely on the fact that the uptake in that locale (4th overall) is more than 5th through 7th combined, so it's clearly a market ripe for adoption.  That wouldn't prevent us from calling it out in the UI tour when we launch the privacy button, but we might as well capture those searches...

Related: what happens when we hit a matching search engine if we add a new default?  We should test to make sure we're correctly migrating and capturing that volume.
I'm working on this list: de, ru, fr, pt-BR, pl, es-ES, ja, it.

I'd like to exclude es-MX and sv-SE, they're working on less flexible toolchains. Which means, they won't get the privacy button and the 33 buzz.

I can add es-CL, though.

Regarding position, after the conversation with dolske and gavin today, gavin tested, and filed bug 1073603. I.e., we can edit region.properties, but it won't affect existing users. For localizations at least, I'd push that out 'til 35 when we can openly talk about the changes we're doing on aurora repos.
Adding en-GB. not sure how close we are, the sign-off is on 32 for desktop.
Note, for doing the same for mobile, we'll need someone to also port and push the desktop search build patch. I'd like to focus on delivering the localized pieces for the next week.
For clarity: nothing should land in a public repo or bug without explicit sign-off from me.  DDG is under some strict conditions with a different partner, so we can't announce/disclose anything until we have signoff from them.
The plan is to land the list.txt additions in the l10n repos by Thursday of next week.

In theory we could do that using a non-descriptive name, since the filename doesn't actually matter. I think we could even clean that up later and rename to duckduckgo.xml after the fact, though I'd need to think a bit more about that.
(In reply to Mike Connor [:mconnor] from comment #23)
> For clarity: nothing should land in a public repo or bug without explicit
> sign-off from me.  DDG is under some strict conditions with a different
> partner, so we can't announce/disclose anything until we have signoff from
> them.

Does this involve only en-US or l10n repos too? In the second case we have an issue with the schedule.

Note that we already have some locales shipping DDG, mostly on mobile
http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=duckduckgo&find=list.txt
Just chatted with dolske. Sounds like we've committed to taking the Makefile hacking approach Pike suggested via email, rather than the land-list.txt-in-l10n-repos approach, so we don't need to rush to get this settled by Thursday or worry about partner disclosure timelines.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #26)
> Just chatted with dolske. Sounds like we've committed to taking the Makefile
> hacking approach Pike suggested via email, rather than the
> land-list.txt-in-l10n-repos approach, so we don't need to rush to get this
> settled by Thursday or worry about partner disclosure timelines.

Agreed. I don't think that I'll many more stakes in the list of locales and the landing patch for this.

Note, this is the plan for 33 for now, I'm not sure how we'll land this for 34 and 35+.
Just got out of a meeting with Chad/mconnor/Pike and others, and we reached the decision to ship DDG with all locales for 33.0.1.
To clarify, that's for desktop specifically. Android may have a different decision, mconnor will followup with Karen.
No longer depends on: 1073212
Attached patch patch (obsolete) — Splinter Review
The icons in this file seem perhaps a bit odd, not sure what those sizes are (need to look into this further). The icon that appears on about:newtab is narrow and off-center (which is only really visible when you mouseover the dropdown):

https://cloudup.com/cAetoibp6GJ
Assignee: mconnor → gavin.sharp
Attachment #8482771 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8502205 - Flags: feedback?(l10n)
Comment on attachment 8502205 [details] [diff] [review]
patch

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

Leaving the f? for the makefile part. About the searchplugin:

1. Missing license header
2. I think this plugin should use the standard structure, so no XML declaration, <SearchPlugin ... > instead of <OpenSearchDescription ... >, no LongName. Not that it impacts functionality, but all other searchplugins use that structure.
3. The first image embeds 16px, 24px and 32px icon, I don't think we need the 24px
4. Do we want to add rel="searchform" (bug 990799)?
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #30)
> Created attachment 8502205 [details] [diff] [review]
> patch
> 
> The icons in this file seem perhaps a bit odd, not sure what those sizes are
> (need to look into this further). The icon that appears on about:newtab is
> narrow and off-center (which is only really visible when you mouseover the
> dropdown):
> 
> https://cloudup.com/cAetoibp6GJ

Those (larger) images need to be fixed.  The icon will center (via css) for the new tab page, I'm guessing they wanted it aligned right. But they shouldn't be using a color for the fill, they could at least use transparent background.

It would be nicer if we could get a wordmark style logo from them for this size, that's the intention of those larger icons.  The DDG logos [1] don't include a horizontal version, perhaps we can request one or set one up for them.

[1] https://onedrive.live.com/?cid=d38146e6f54d5475&id=D38146E6F54D5475!139&ithint=folder,.svg&authkey=!AGg0Aw4zd63lr3o
I don't think we can fit a readable wordmark into the 65x26 format.  Right-aligned instead of centred seems better to me.

FWIW, I thought was transparent, but I didn't inspect the icons much, just threw them all into a working descriptor.  We can strip 24x24, I didn't have the right tools on hand to strip it.  If we can do that, great, or I can ask them.
Attached file Test Images (obsolete) —
Bryan, images are already transparent.
Attached image .ico with only 16px and 32px (obsolete) —
.ico file created using 16px and 32px images from original image
Attachment #8502205 - Flags: feedback?(l10n) → feedback+
QA Contact: petruta.rasa
Okay, we're clear to land this tomorrow.  We still need a Fennec version of the patch in bug 883254, and we should clean up the patch to have the new .ico from flod.
Attached patch updated patchSplinter Review
Changes since last patch:
- updated "16x16" image to use flod's version from attachment 8503282 [details]
- removed unused "LongName" attribute

Changes not made:
- didn't add a license header (need to followup on this, not sure it's the right thing to do)
- didn't revert to "MozSearch" format, stuck with OpenSearch (I see no reason not to just use OpenSearch here)
- didn't add rel=searchform, since there's not much benefit at this point given the engine parameters. Can always revisit later.
- didn't fix the off-center images, we can followup to fix that
Attachment #8502205 - Attachment is obsolete: true
Attachment #8503280 - Attachment is obsolete: true
Attachment #8503282 - Attachment is obsolete: true
Attachment #8512367 - Flags: review?(dolske)
mconnor was asking me whether this Makefile-hacking approach was feasible for Fennec.  I don't see why not:

* land (a modified) ddg.xml into mobile/locales/en-US/searchplugins/ddg.xml
* at http://mxr.mozilla.org/mozilla-central/source/mobile/locales/Makefile.in#94, append ddg

mconnor assures me that every existing locale falls back to en-US/searchplugins/{google,bing}.xml, so we should also fall back to en-US/searchplugins/{ddg}.xml.
(In reply to Nick Alexander :nalexander from comment #38)
> mconnor was asking me whether this Makefile-hacking approach was feasible
> for Fennec.  I don't see why not:
> 
> * land (a modified) ddg.xml into mobile/locales/en-US/searchplugins/ddg.xml
> * at
> http://mxr.mozilla.org/mozilla-central/source/mobile/locales/Makefile.in#94,
> append ddg
> 
> mconnor assures me that every existing locale falls back to
> en-US/searchplugins/{google,bing}.xml, so we should also fall back to
> en-US/searchplugins/{ddg}.xml.

Doesn't work. See bug 883254. When I tried the above, the DuckDuckGo engine did not even show up for en-US. We needed to add it to list.txt to get it into our jar.
I just realized that we might have a problem with locales already shipping DDG. 

On desktop it's only gd, but we have more on mobile (cy, es-MX, gd, ml, ta). Are these locales going to have DDG twice (the existing duckduckgo.xml, plus new ddg.xml added at build-time)?

Is the build change going to land on all branches, or are we going to add DDG to list.txt for 34 and later?
Comment on attachment 8512367 [details] [diff] [review]
updated patch

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

I'm confused about how this actually works.  We want to do a similar thing in Fennec, and I think we need to make accommodations for altering the list.

::: browser/locales/Makefile.in
@@ +70,5 @@
>  
>  ifeq ($(MOZ_WIDGET_TOOLKIT) $(DIST_SUBDIR),windows metro)
>  SEARCHPLUGINS_NAMES = $(shell cat $(call MERGE_FILE,/searchplugins/metrolist.txt))
>  else
> +SEARCHPLUGINS_NAMES = $(shell cat $(call MERGE_FILE,/searchplugins/list.txt)) ddg

How does this work in the wild?  list.txt is packaged in the omnijar, and it's extracted at [1].  It doesn't look like list.txt is constructed from the list of names; it looks like it's straight copied via [2].

[1] http://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#3603

[2] http://dxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in#43
On desktop, we're loading the plugins from disk, they're in locations like /Applications/FirefoxNightly.app//Contents/Resources/browser/searchplugins/google.xml.

The list.txt is only needed for mobile, as we load from the jar there.
(In reply to Francesco Lodolo [:flod] from comment #40)
> I just realized that we might have a problem with locales already shipping
> DDG. 
> 
> On desktop it's only gd, but we have more on mobile (cy, es-MX, gd, ml, ta).
> Are these locales going to have DDG twice (the existing duckduckgo.xml, plus
> new ddg.xml added at build-time)?

The gd duckduckgo.xml has a shortname of "DuckDuckGo", which matches mine, so only one will be loaded by the search service. Not sure which one, but I think it's not a huge deal if it gets the old one rather than this one. We can followup to remove the duplication later. Shouldn't be an issue for desktop.

> Is the build change going to land on all branches, or are we going to add
> DDG to list.txt for 34 and later?

We'll land this patch on all branches now. We may change how this works later on mozilla-central.
(In reply to Nick Alexander :nalexander from comment #41)
> How does this work in the wild?  list.txt is packaged in the omnijar, and
> it's extracted at [1].  It doesn't look like list.txt is constructed from
> the list of names; it looks like it's straight copied via [2].
> 
> [1]
> http://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/
> nsSearchService.js#3603

As Axel said, _findJAREngines isn't called on desktop (browser.search.loadFromJars is not set there). We just load any files on disk. So we don't need to worry about having list.txt actually contain this engine, but mobile does if it wants the same behavior.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #44)
> (In reply to Nick Alexander :nalexander from comment #41)
> > How does this work in the wild?  list.txt is packaged in the omnijar, and
> > it's extracted at [1].  It doesn't look like list.txt is constructed from
> > the list of names; it looks like it's straight copied via [2].
> > 
> > [1]
> > http://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/
> > nsSearchService.js#3603
> 
> As Axel said, _findJAREngines isn't called on desktop
> (browser.search.loadFromJars is not set there). We just load any files on
> disk. So we don't need to worry about having list.txt actually contain this
> engine, but mobile does if it wants the same behavior.

Awesome, thanks for the corrections everybody.
Comment on attachment 8512367 [details] [diff] [review]
updated patch

rs=me
Attachment #8512367 - Flags: review?(dolske) → review+
Tested alder 2014-10-29 builds under Mac OSX 10.9.5, Ubuntu 14.04 32-bit and Win 7 64-bit.

Notes:
1. On all en-US builds, DDG is placed on the 5th position 

2. DDG add-on remains the first one from https://addons.mozilla.org/en-US/firefox/search/?atype=4 list (Manage Search Engine List -> Get more search engines)

3. Some later fixes on Search component, such as bug 1032324 that only reached 35, may fix this remaining work: "didn't fix the off-center images, we can followup to fix that" and another use case (opening Fx 34 with a profile, adding DDG add-on and then opening alder build with the same profile, will display DuckDuckGo string inside the search text field)

Should I test any locales after landing? Which ones?
1. That's expected. Only 3 (Google, Yahoo, Bing) have a search order constraint, the rest is just in alphabetical order (amazon, ddg, etc.).

3. Bug 1032324 only affects searchplugins that are missing the extra images, it's not the case for DDG.

I'll test Italian as soon as I have a build (still missing for mac).
(In reply to Francesco Lodolo [:flod] from comment #50)
> I'll test Italian as soon as I have a build (still missing for mac).

Was finally able to test Mac build for Italian, everything works as expected.
Depends on: 1091048
Verified 33.1 candidate builds using en-US and several locales (gd included) under Mac OSX 10.9.5, Ubuntu 12.04 32-bit and Win 7 64-bit. No new issues found, this feature is ready for release.
The changeset has been linked and is public https://hg.mozilla.org/releases/mozilla-release/rev/0746f89a5aee, is there any reason to keep this bug confidential?
(In reply to Aaron Train [:aaronmt] from comment #53)
> The changeset has been linked and is public
> https://hg.mozilla.org/releases/mozilla-release/rev/0746f89a5aee, is there
> any reason to keep this bug confidential?

There are details in this bug that provide more "meat" for potential articles than that commit link. We'll open this release week.
Added to the release notes "Added DuckDuckGo search provider"
From today's stand-up, DDG should land on Gum today and merge to Aurora tomorrow (Friday). This change should land on m-c and beta on Monday.
Attachment #8512367 - Flags: approval-mozilla-beta+
Attachment #8512367 - Flags: approval-mozilla-aurora+
Note: we're still missing this in mozilla-central.
Flags: qe-verify+
Group: mozilla-employee-confidential
Duplicate of this bug: 697326
Amy particular reason this has yet to land on m-c Nightly builds ?
Flags: needinfo?(gavin.sharp)
Attachment #8503282 - Attachment mime type: application/ico → application/x-icon
Attachment #8503282 - Attachment mime type: application/x-icon → image/x-icon
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #61)
> Any particular reason this has yet to land on m-c Nightly builds ?
Since the hacky workarounds aren't needed for m-c Nightly, a different patch is needed.
(In reply to Philip Chee from comment #62)
> (In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #61)
> > Any particular reason this has yet to land on m-c Nightly builds ?
> Since the hacky workarounds aren't needed for m-c Nightly, a different patch
> is needed.

I agree that this needs to be properly fixed on the long run (add the searchplugin to list.txt), but keep in mind that l10n will require some time to manually fix this when you drop the hack, with the risk of dropping DDG completely from one version to another.
https://hg.mozilla.org/integration/fx-team/rev/bf58b2c55797
Flags: needinfo?(gavin.sharp)
Target Milestone: --- → Firefox 36
I filed bug 1105092 on the proper addition.
https://hg.mozilla.org/mozilla-central/rev/bf58b2c55797
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Verified that DDG is added by default to search engine list using Firefox 35 beta 1, latest Dev Edition 36.0a2 and latest Nightly 37.0a1 under all platforms.
You need to log in before you can comment on or make changes to this bug.