603298 - Add Bing to the default en-US search plugins; remove Answers.com and CC

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Kev Needham [:kev]]

Attachment: Add Bing to default search plugins in third position, remove Answers & CC from default list

Add the Bing search plugin to the default list of providers in the search bar. The plugin will be placed in the third position of the Search Bar drop down after Google and Yahoo!. Remove Answers.com and Creative Commons search plugins from the default set included in en-US. (NB: per l10n, leave the Answers and CC plugin files in place for locales that may still use them)

The expected search bar query for builds that incorporate official branding generated by this plugin is: http://www.bing.com/search?q={searchTerms}&form=MOZSBR&pc=MOZI

The expected search bar query for builds that do not incorporate official branding generated by this plugin is: http://www.bing.com/search?q={searchTerms}&form=MOZSBR

Comment 1

[Staś Małolepszy :stas]

Comment on attachment 482216 [details] [diff] [review]
Add Bing to default search plugins in third position, remove Answers & CC from default list

>diff -r 4f238d3d3e6a browser/locales/en-US/chrome/browser-region/region.properties
>+browser.search.order.3=Bing

I don't think this will work without a

pref("browser.search.order.3", "chrome://browser-region/locale/region.properties");

in http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/firefox-l10n.js or http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#330. The latter would be better, since it's global, and will not require any additional changes by other locales to their respective firefox-l10n.js if they wish to have Bing as an explicit 3rd choice.

Comment 2

[Kev Needham [:kev]]

Attachment: Updated patch to address comment #1

Per comment #1, revised patch to add search order pref in firefox.js

Comment 3

[Staś Małolepszy :stas]

Comment on attachment 482240 [details] [diff] [review]
Updated patch to address comment #1

r=me, thanks Kev!

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

We'll want this in the b7 builds, at least in en-US.

Comment 5

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

Comment on attachment 482240 [details] [diff] [review]
Updated patch to address comment #1

Couple of minor issues here:
- some tabs crept in, and line wrapping seems to be wonky (can be fixed on checkin)
- condition="pref" MozParams don't handle non-existent pref values, so we need to either fix the search service so that they do, or be sure that the pref is always present (as it is, I think this will break unbranded builds because browser.search.param.ms-pc doesn't exist). Note also that the current behavior of MozParams doesn't allow omitting the parameter entirely, so this will result in either "pc=" or "pc=MOZI" being passed. That may not matter, but we can change that behavior as well if needed.
- is the "language" param for the suggest URL needed? It isn't included for the other URLs, and its value will be one of our locale codes, which may not be what they're expecting...

Comment 6

[Rimas Kudelis [use needinfo to ping]]

Is there any chance to supply application's locale to Bing as a parameter? Were it possible, it would most probably nullify the need for localizers to have a slightly modified clone of bing.xml in their localizations. We could use the en-US file instead, like is done with Google.

Comment 7

[Kev Needham [:kev]]

(In reply to comment #5)
> 
> Couple of minor issues here:
> - some tabs crept in, and line wrapping seems to be wonky (can be fixed on
> checkin)
> - condition="pref" MozParams don't handle non-existent pref values, so we need
> to either fix the search service so that they do, or be sure that the pref is
> always present (as it is, I think this will break unbranded builds because
> browser.search.param.ms-pc doesn't exist). Note also that the current behavior
> of MozParams doesn't allow omitting the parameter entirely, so this will result
> in either "pc=" or "pc=MOZI" being passed. That may not matter, but we can
> change that behavior as well if needed.
> - is the "language" param for the suggest URL needed? It isn't included for the
> other URLs, and its value will be one of our locale codes, which may not be
> what they're expecting...

I'll fix the whitespace, that's my bad. 

When I tested the mozparams with Minefield, the pc attribute was just ignored, so no pc=MOZI was passed, which is desired. If there's a better way to handle it, then I'll put it in. How do we handle the Yahoo! case, which is similar, iirc?

The "language" param was included because that's how the Bing team has written the plugin they make available to us. I'll double-check with them ASAP.

(In reply to comment #6)
> Is there any chance to supply application's locale to Bing as a parameter? Were
> it possible, it would most probably nullify the need for localizers to have a
> slightly modified clone of bing.xml in their localizations. We could use the
> en-US file instead, like is done with Google.

We can review case-by-case, but the Bing team has stated that the default plugin should work properly in all locales (e.g. no modifications are necessary, and the default plugin _should_ be used in all cases). We'd like localizers to test this, of course, and let us know if they're seeing sub-optimal results with the default.

Comment 8

[Rimas Kudelis [use needinfo to ping]]

Oh, nice to know that it should work.

One more thing: I don't see where's the rationale of removing CC. It would be nice if there was an explanation somewhere, like that in bug 585741 for answers.com

Comment 9

[Rimas Kudelis [use needinfo to ping]]

By the way, why is line 21 of bing.xml split? Isn't that a mistake?

Comment 10

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

(In reply to comment #7)
> When I tested the mozparams with Minefield, the pc attribute was just ignored,
> so no pc=MOZI was passed, which is desired.

Indeed! I misread the code, that is already handled.

One additional thing I just thought of: because of the way we save engine ordering, the default order won't apply to existing profiles that have either installed an engine, removed an engine, or moved an engine (any engine). For those users, Bing will be added to the end of the list. I don't think there's much that we can do about that, unless we write additional code to completely reset ordering on upgrade (or potentially something smarter, like only resetting if all of the engines are defaults).

Comment 11

[Rimas Kudelis [use needinfo to ping]]

(In reply to comment #7)
> (In reply to comment #6)
> > Is there any chance to supply application's locale to Bing as a parameter? Were
> > it possible, it would most probably nullify the need for localizers to have a
> > slightly modified clone of bing.xml in their localizations. We could use the
> > en-US file instead, like is done with Google.
> 
> We can review case-by-case, but the Bing team has stated that the default
> plugin should work properly in all locales (e.g. no modifications are
> necessary, and the default plugin _should_ be used in all cases). We'd like
> localizers to test this, of course, and let us know if they're seeing
> sub-optimal results with the default.

I've just tested this. Bing is shown in English locale, and I still have to click a link at the top of the page (TWICE!) to switch language to Lithuanian, which is then stored in a cookie. To me, this feels pretty suboptimal.

Comment 12

[Staś Małolepszy :stas]

(In reply to comment #11)

> I've just tested this. Bing is shown in English locale, and I still have to
> click a link at the top of the page (TWICE!) to switch language to Lithuanian,
> which is then stored in a cookie. To me, this feels pretty suboptimal.

I wasn't able to reproduce this. I'm currently in Switzerland, and using English Mac OS X and a clean en-US profile with "Lithuanian" as my top language preference both the plugin's results page and the website are displayed to me in Lithuanian. To me it looks like they got the language detection just right. Rimas, would mind trying on a new profile?

Comment 13

[Rimas Kudelis [use needinfo to ping]]

(In reply to comment #12)
> (In reply to comment #11)
> 
> > I've just tested this. Bing is shown in English locale, and I still have to
> > click a link at the top of the page (TWICE!) to switch language to Lithuanian,
> > which is then stored in a cookie. To me, this feels pretty suboptimal.
> 
> I wasn't able to reproduce this. I'm currently in Switzerland, and using
> English Mac OS X and a clean en-US profile with "Lithuanian" as my top language
> preference both the plugin's results page and the website are displayed to me
> in Lithuanian. To me it looks like they got the language detection just right.
> Rimas, would mind trying on a new profile?

Weird. Win7, Lithuanian Firefox 3.6.10, XML file from the attachment, Lithuanian is a top preferred language. Bing opens its results in English to me.

Comment 14

[Rimas Kudelis [use needinfo to ping]]

Attachment: English interface, while should be Lithuanian

Well, this is what I see. And this is a completely fresh profile, created specifically for the purpose of testing bing.xml.

Comment 15

[Rimas Kudelis [use needinfo to ping]]

Hm, interesting: I just copied a Bing link from Firefox to Minefield, and it indeed results in localized Lithuanian interface. I guess guys at Microsoft tweak their output per-browser/version again? What a crappy practice...

Comment 16

[Rimas Kudelis [use needinfo to ping]]

Doh, it seems it was too early to speak. Clearing all cookies and loading that same URL again results in English page even in Minefield.

Comment 17

[Axel Hecht [:Pike]]

(In reply to comment #7)
> The "language" param was included because that's how the Bing team has written
> the plugin they make available to us. I'll double-check with them ASAP.

This seems to be a remaining open question on the patch at hand? Kev, any updates on that?

Rimas, mind filing a separate bug on your issue? That could be something weird as a Lithuanian datacenter lagging on software updates or whatnot ;-). If you do, could you reproduce your behaviour on a cookie free profile with logs? https://developer.mozilla.org/en/HTTP_Logging works in production builds, no debug builds needed. CC me, stas, kev on that bug?


PS: We should really get this landed, localizers started to change list.txt and obviously break at build time now.

Comment 18

[Kev Needham [:kev]]

First, let's move the Lithuanian issue over to the an LT bug, it doesn't affect this particular case, and I'm glad we are doing checks. Assumptions like those in comment #15 are pure speculation, so we'll get in touch with MS on Monday and see what's what. Worst case, we have a custom plugin IF we can't get the issue addressed server-side, and you want to have Bing in LT. Rimas, please file bug 604979 under l10n, and I don't think I'd accept it as a blocker for this bug unless it affects en-US as well.

WRT comment #10, if that's the way ordering in product works, then that's the way it works :) I can see adding code being more effort than it's worth, and I think it's acceptable as-is.

WRT comment #17, the search suggest doesn't currently perform do geo-ip lookups, so unless a language is specified, your results will be in english. We're still confirming that with the server team, and I'll make sure I have an answer by CoB tomorrow.

I'll update the patch to clean it up, as well.

Comment 19

[Kev Needham [:kev]]

Attachment: Updated patch, corrects whitespace issues

Patch to correct whitespace issue. Will hold on review request until language param in search suggest is confirmed, no later than CoB Mon Oct 18.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Any update here, Kev?

Comment 21

[Kev Needham [:kev]]

Comment on attachment 483826 [details] [diff] [review]
Updated patch, corrects whitespace issues

Good to go. Apologies for the delay, just finished up with discussion 10 min ago, and the language codes should work as expected. Search suggest doesn't perform geo-ip lookups, and needs a param, otherwise it'll return results in English. Will ask for r+ from stas and away we go.

Comment 22

[Kev Needham [:kev]]

Attachment: Updated Bing patch with context, to correct whitespace issues.

per discussion in #planning, I hadn't created the diff quite right. updated patch to address whitespace issues.

Comment 23

[Kev Needham [:kev]]

Attachment: Updated Bing patch with context, to correct whitespace issues

Because I am awesome, I included an eBay modification in the previous patch that should not be there. Here is yet another patch to correct the whitespace/linebreak fun in my previous patch. I am sincerely hoping this is it.

Sincerely.

Comment 24

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

Comment on attachment 484494 [details] [diff] [review]
Updated Bing patch with context, to correct whitespace issues

(In reply to comment #21)
> Good to go. Apologies for the delay, just finished up with discussion 10 min
> ago, and the language codes should work as expected. Search suggest doesn't
> perform geo-ip lookups, and needs a param, otherwise it'll return results in
> English.

I tested the suggest URL with "de", "fr", "es", and "pl" - they all seemed to return the same results as en-US, so I'm not sure how effective this really is (I guess they may enable localized suggestions later?). A bogus locale code returns no suggestions, though, so there's some indication that they do recognize those codes at least.

Should we send "pc" for the suggest URL as well? ("OSDJAS" doesn't seem Mozilla-specific like the other "form" values, but I guess they maybe care less about tracking that one?).

I should have realized this earlier, but the application/x-moz-keywordsearch URL won't actually ever be used, since we only ever use the as-shipped default plugin for keyword search. Doesn't really hurt to leave it in, though.

We may have to do some packaging tricks if we don't end up removing both creativecommons and answers in all locales, but we can figure that out separately. If we do remove them everywhere, I think we need to add them to removed-files.in.

Comment 25

[Kev Needham [:kev]]

> Should we send "pc" for the suggest URL as well? ("OSDJAS" doesn't seem
> Mozilla-specific like the other "form" values, but I guess they maybe care less
> about tracking that one?).

No. Search suggests are something to prime the query only at this point, and there's no need (currently) to add our partner code to it. The "OSDJAS" form code just identifies it as coming from an OpenSearch plugin.

> I should have realized this earlier, but the application/x-moz-keywordsearch
> URL won't actually ever be used, since we only ever use the as-shipped default
> plugin for keyword search. Doesn't really hurt to leave it in, though.

I was hoping the follow whatever the default search (or make it simple to change location bar search to a search plugin) would make it into the product, but alas. Still think we need to change that in the near future if it's not, so I'd like to leave it in there, if possible.

> We may have to do some packaging tricks if we don't end up removing both
> creativecommons and answers in all locales, but we can figure that out
> separately. If we do remove them everywhere, I think we need to add them to
> removed-files.in.

Axel, any thoughts here? That's beyond my ken, for sure.

Comment 26

[Staś Małolepszy :stas]

I did some more testing of the suggest URL. I'm currently in France and here's what I was able to deduce:

- the 'language' parameter needs to be of the 'ab-CD' form. "fr-FR" works but "fr" doesn't. This is a problem for us, since for most of the locales, we use  two-letter codes (e.g. 'fr', 'de', 'pl').

- if the language parameter is not recognized, the suggestion service will do a geo-ip lookup. I tested this from France and via a US-based VPN (see tests below).

- the Accept-Language header is not taken into account.

- I wasn't able to test all locales, but I didn't manage to find many ab-CD locale codes that work properly. Among the working ones were fr-FR, en-US, en-GB, en-AU and other en-*. I checked it-IT, de-DE, es-ES, pl-PL and none of those returns anything at the moment. As I mentioned above, even if they did return something, we still need the suggest URL to support short locale codes.


$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS"
> ["tabl",["table de multiplication","tablature","table basse","tablature guitare","tableau noir","table des calories","tableau de conversion","table à langer"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=xx"
> ["tabl",["table de multiplication","tablature","table basse","tablature guitare","tableau noir","table des calories","tableau de conversion","table à langer"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=xx-XX"
> ["tabl",["table de multiplication","tablature","table basse","tablature guitare","tableau noir","table des calories","tableau de conversion","table à langer"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=pl"
> ["tabl",["table de multiplication","tablature","table basse","tablature guitare","tableau noir","table des calories","tableau de conversion","table à langer"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=pl-PL"
> ["tabl",[]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=en-US"
> ["tabl",["tablet pc","tablecloths","table lamps","table linens","table of elements","table runners","table saws","table rock lake"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=en-GB"
> ["tabl",["table lamps","table table","table tennis games","table decorations","tablet pc","table mountain","tablecloths","table tennis"]]

$ curl "http://api.bing.com/osjson.aspx?query=tabl&form=OSDJAS&language=en-AU"
> ["tabl",["table eight","tablelands regional council","table lamps","tablet pc","table decorations","table tennis victoria","table tennis","tablet hotels"]]

It might be a good idea to reach out to Bing and ask to clarify/fix the issues I listed above. Presently, for most of the locales the suggest service will do a geo-ip lookup (since it will not recognize a two-letter locale code) and will offer suggestions based on the region, not the language preference of the user.

Comment 27

[Axel Hecht [:Pike]]

No packaging magic required, as we're removing answers and cc only from list.txt, but we're not removing answers.xml nor creativecommons.xml, exactly for that purpose.

Comment 28

[Kev Needham [:kev]]

(In reply to comment #26)
> I did some more testing of the suggest URL. I'm currently in France and here's
> what I was able to deduce:

Ok, what's the preferred avenue here, then? We can take it out and work with Bing to use accept-lang or add our character sets, but there's zero guarantee that will happen. From the "pl" results, I'm inclined to remove it and let geo-ip figure it out. I've sent a request for clarification. Thanks for the extra hammering, stas.


(In reply to comment #27)
> No packaging magic required, as we're removing answers and cc only from
> list.txt, but we're not removing answers.xml nor creativecommons.xml, exactly
> for that purpose.

Awesome. Thanks, Axel.

Comment 29

[Staś Małolepszy :stas]

(In reply to comment #28)

> Ok, what's the preferred avenue here, then? We can take it out and work with
> Bing to use accept-lang or add our character sets, but there's zero guarantee
> that will happen. From the "pl" results, I'm inclined to remove it and let
> geo-ip figure it out. I've sent a request for clarification. Thanks for the
> extra hammering, stas.

I think the best way would be for Bing to look at the Accept-Language header and return suggestions based on it. It would keep the suggest and actual search experience consistent. 

Next on the list would be the 'language' GET parameter (but accepting Mozilla-style two-letter-long locale codes). 

I'd put geo-ip at the last position because a user's location is not a consumer choice he or she makes.

Comment 30

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

(In reply to comment #27)
> No packaging magic required, as we're removing answers and cc only from
> list.txt, but we're not removing answers.xml nor creativecommons.xml, exactly
> for that purpose.

The search service doesn't read list.txt - it loads all engines present on disk. That means that without adding the files to removed-files.in, they won't be removed on a complete update. So we do need some sort of packaging change if we want to ensure that they are removed for existing users.

Comment 31

[Kev Needham [:kev]]

(In reply to comment #30)
> The search service doesn't read list.txt - it loads all engines present on
> disk. That means that without adding the files to removed-files.in, they won't
> be removed on a complete update. So we do need some sort of packaging change if
> we want to ensure that they are removed for existing users.

Are there any suggestions for how we should handle this for en-US, then? I've got the reply from MS, and the language param can be dropped. I'll update the patch, but if we can get the packaging issue addressed, that'd be great.

Comment 32

[Kev Needham [:kev]]

Attachment: Drop "language" attrib from search suggest.

per comment #26 and comment #29, this patch removes the language attribute from the search suggest. the search suggest will rely on geo-ip, and we'll work with the Bing team to try and get accept-language support into the API server (which won't require any updates to the change here) which delivers the Bing search suggest results.

Comment 33

[Axel Hecht [:Pike]]

(In reply to comment #31)
The issue about removing CC and answers just for some locales should be an RFE on the installer/updater code, IMHO. Makes sense?

Comment 34

[Mike Beltzner [:beltzner, not reading bugmail]]

Stas: can we get a review here, please?

Comment 35

[Staś Małolepszy :stas]

Comment on attachment 484904 [details] [diff] [review]
Drop "language" attrib from search suggest.

This looks good, r=me. Thanks again, Kev!

Comment 36

[Axel Hecht [:Pike]]

Pushed to both default and branch, http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=092596c1faef.

If we need to get the answers/cc removal for existing installs, file a follow up bug on that?

Comment 37

[Jim Jeffery not reading bug-mail 1/2/11]

The bug title says 'remove answers & CC' , I just installed the latest hourly, and Bing is there, but the other that were to be removed are still there....

Using hourly from here: 
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1287853929/

Comment 38

[Axel Hecht [:Pike]]

See comment 36, and before.

Comment 39

[pal-moz]

(In reply to comment #37)
> The bug title says 'remove answers & CC' , I just installed the latest hourly,
> and Bing is there, but the other that were to be removed are still there....
> 
> Using hourly from here: 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1287853929/

Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101024 Firefox/4.0b8pre ID:20101024042142
(Nightly)

both are deleted.